From cefc999c6c23e6e1214914e0ae48725cd45d8802 Mon Sep 17 00:00:00 2001 From: "wenda.ni" Date: Tue, 29 Sep 2020 19:35:01 +0000 Subject: [PATCH 1/7] Restore eth0 up in post-test clean up Signed-off-by: Wenda Ni --- tests/test_portchannel.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_portchannel.py b/tests/test_portchannel.py index 96505cd1c8..4e03a81aac 100644 --- a/tests/test_portchannel.py +++ b/tests/test_portchannel.py @@ -264,6 +264,10 @@ def test_Portchannel_oper_down(self, dvs, testlog): tbl._del("PortChannel004") time.sleep(1) + # Restore eth0 up + dvs.servers[0].runcmd("ip link set up dev eth0") + time.sleep(1) + # Add Dummy always-pass test at end as workaroud # for issue when Flaky fail on final test it invokes module tear-down before retrying From b4a10ad2f31f1c3926c27a4cc9cdc6e58ad4104c Mon Sep 17 00:00:00 2001 From: Wenda Ni Date: Mon, 19 Oct 2020 11:07:49 +0800 Subject: [PATCH 2/7] Sub interface inherits parent port mtu changes 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 7f06c73bf9..ebf3e57b07 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -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; @@ -2318,6 +2345,8 @@ void PortsOrch::doPortTask(Consumer &consumer) { gIntfsOrch->setRouterIntfsMtu(p); } + // Sub interfaces inherit parent physical port mtu + updateChildPortsMtu(p, mtu); } else { @@ -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)) diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index 806a63e53a..4d7ebb25b1 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 e4d6adb950..124514ec07 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 From 14819682ee21f3719da275786e767812049e592a Mon Sep 17 00:00:00 2001 From: Wenda Ni Date: Wed, 21 Oct 2020 09:08:28 +0800 Subject: [PATCH 3/7] ecmp hardware convergence acceleration in the event of parent port oper status change Signed-off-by: Wenda Ni --- orchagent/neighorch.cpp | 11 +- orchagent/portsorch.cpp | 26 ++- tests/test_sub_port_intf.py | 372 +++++++++++++++++++++++++++++++++++- 3 files changed, 400 insertions(+), 9 deletions(-) diff --git a/orchagent/neighorch.cpp b/orchagent/neighorch.cpp index 5ba5ec4e27..eb12b2729c 100644 --- a/orchagent/neighorch.cpp +++ b/orchagent/neighorch.cpp @@ -38,6 +38,15 @@ bool NeighOrch::addNextHop(const IpAddress &ipAddress, const string &alias) ipAddress.to_string().c_str(), alias.c_str()); return false; } + if (p.m_type == Port::SUBPORT) + { + if (!gPortsOrch->getPort(p.m_parent_port_id, p)) + { + SWSS_LOG_ERROR("Neighbor %s seen on sub interface %s whose parent port doesn't exist", + ipAddress.to_string().c_str(), alias.c_str()); + return false; + } + } NextHopKey nexthop = { ipAddress, alias }; assert(!hasNextHop(nexthop)); @@ -90,7 +99,7 @@ bool NeighOrch::addNextHop(const IpAddress &ipAddress, const string &alias) gFgNhgOrch->validNextHopInNextHopGroup(nexthop); // For nexthop with incoming port which has down oper status, NHFLAGS_IFDOWN - // flag Should be set on it. + // flag should be set on it. // This scenario may happen under race condition where buffered neighbor event // is processed after incoming port is down. if (p.m_oper_status == SAI_PORT_OPER_STATUS_DOWN) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index c6aa40ecc6..3dc930e5dd 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -635,7 +635,7 @@ bool PortsOrch::addSubPort(Port &port, const string &alias, const bool &adminUp, } if (vlan_id > MAX_VALID_VLAN_ID) { - SWSS_LOG_ERROR("sub interface %s Port object creation failed: invalid VLAN id %u", alias.c_str(), vlan_id); + SWSS_LOG_ERROR("Sub interface %s Port object creation failed: invalid VLAN id %u", alias.c_str(), vlan_id); return false; } @@ -2838,8 +2838,6 @@ void PortsOrch::doLagTask(Consumer &consumer) continue; } - gNeighOrch->ifChangeInformNextHop(alias, - (operation_status == "up")); Port lag; if (getPort(alias, lag)) { @@ -2868,11 +2866,23 @@ void PortsOrch::doLagTask(Consumer &consumer) } else { - if (!operation_status.empty()) { l.m_oper_status = string_oper_status.at(operation_status); m_portList[alias] = l; + + bool isUp = operation_status == "up" ? true : false; + if (!gNeighOrch->ifChangeInformNextHop(alias, isUp)) + { + SWSS_LOG_WARN("Inform nexthop operation failed for interface %s", alias.c_str()); + } + for (const auto &child_port : l.m_child_ports) + { + if (!gNeighOrch->ifChangeInformNextHop(child_port, isUp)) + { + SWSS_LOG_WARN("Inform nexthop operation failed for sub interface %s", child_port.c_str()); + } + } } if (operation_status_changed) { @@ -2881,6 +2891,7 @@ void PortsOrch::doLagTask(Consumer &consumer) update.operStatus = string_oper_status.at(operation_status); notify(SUBJECT_TYPE_PORT_OPER_STATE_CHANGE, static_cast(&update)); } + if (mtu != 0) { l.m_mtu = mtu; @@ -4140,6 +4151,13 @@ void PortsOrch::updatePortOperStatus(Port &port, sai_port_oper_status_t status) { SWSS_LOG_WARN("Inform nexthop operation failed for interface %s", port.m_alias.c_str()); } + for (const auto &child_port : port.m_child_ports) + { + if (!gNeighOrch->ifChangeInformNextHop(child_port, isUp)) + { + SWSS_LOG_WARN("Inform nexthop operation failed for sub interface %s", child_port.c_str()); + } + } PortOperStateUpdate update = {port, status}; notify(SUBJECT_TYPE_PORT_OPER_STATE_CHANGE, static_cast(&update)); diff --git a/tests/test_sub_port_intf.py b/tests/test_sub_port_intf.py index 124514ec07..c7afe2de81 100644 --- a/tests/test_sub_port_intf.py +++ b/tests/test_sub_port_intf.py @@ -1,7 +1,9 @@ import json +import time from dvslib.dvs_common import wait_for_result from dvslib.dvs_database import DVSDatabase +from swsscommon import swsscommon DEFAULT_MTU = "9100" @@ -14,9 +16,16 @@ STATE_INTERFACE_TABLE_NAME = "INTERFACE_TABLE" APP_INTF_TABLE_NAME = "INTF_TABLE" +APP_ROUTE_TABLE_NAME = "ROUTE_TABLE" +APP_PORT_TABLE_NAME = "PORT_TABLE" +APP_LAG_TABLE_NAME = "LAG_TABLE" ASIC_RIF_TABLE = "ASIC_STATE:SAI_OBJECT_TYPE_ROUTER_INTERFACE" ASIC_ROUTE_ENTRY_TABLE = "ASIC_STATE:SAI_OBJECT_TYPE_ROUTE_ENTRY" +ASIC_NEXT_HOP_TABLE = "ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP" +ASIC_NEXT_HOP_GROUP_TABLE = "ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP" +ASIC_NEXT_HOP_GROUP_MEMBER_TABLE = "ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER" +ASIC_NEXT_HOP_TABLE = "ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP" ADMIN_STATUS = "admin_status" @@ -45,6 +54,24 @@ def connect_dbs(self, dvs): self.state_db = dvs.get_state_db() dvs.setup_db() + def get_parent_port_index(self, port_name): + if port_name.startswith(ETHERNET_PREFIX): + idx = int(port_name[len(ETHERNET_PREFIX):]) + else: + assert port_name.startswith(LAG_PREFIX) + idx = int(port_name[len(LAG_PREFIX):]) + return idx + + def set_parent_port_oper_status(self, dvs, port_name, status): + if port_name.startswith(ETHERNET_PREFIX): + srv_idx = self.get_parent_port_index(port_name) // 4 + dvs.servers[srv_idx].runcmd("ip link set dev eth0 " + status) + else: + assert port_name.startswith(LAG_PREFIX) + dvs.runcmd("bash -c 'echo " + ("1" if status == "up" else "0") + \ + " > /sys/class/net/" + port_name + "/carrier'") + time.sleep(1) + def set_parent_port_admin_status(self, dvs, port_name, status): fvs = {ADMIN_STATUS: status} @@ -55,10 +82,11 @@ 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 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'") + if port_name.startswith(ETHERNET_PREFIX): + self.set_parent_port_oper_status(dvs, port_name, "down") + self.set_parent_port_oper_status(dvs, port_name, "up") + else: + self.set_parent_port_oper_status(dvs, port_name, "up") def create_sub_port_intf_profile(self, sub_port_intf_name): fvs = {ADMIN_STATUS: "up"} @@ -98,6 +126,29 @@ def get_newly_created_oid(self, table, old_oids): oid = [ids for ids in new_oids if ids not in old_oids] return oid[0] + def get_ip_prefix_nhg_oid(self, ip_prefix): + def _access_function(): + route_entry_found = False + + raw_route_entry_keys = self.asic_db.get_keys(ASIC_ROUTE_ENTRY_TABLE) + for raw_route_entry_key in raw_route_entry_keys: + route_entry_key = json.loads(raw_route_entry_key) + if route_entry_key["dest"] == ip_prefix: + route_entry_found = True + break + + return (route_entry_found, raw_route_entry_key) + + (route_entry_found, raw_route_entry_key) = wait_for_result(_access_function, DVSDatabase.DEFAULT_POLLING_CONFIG) + + fvs = self.asic_db.get_entry(ASIC_ROUTE_ENTRY_TABLE, raw_route_entry_key) + + nhg_oid = fvs.get( "SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID", "") + assert nhg_oid != "" + assert nhg_oid != "oid:0x0" + + return nhg_oid + def check_sub_port_intf_key_existence(self, db, table_name, key): db.wait_for_matching_keys(table_name, [key]) @@ -468,6 +519,319 @@ def test_sub_port_intf_mtu(self, 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) + def create_nhg_router_intfs(self, dvs, parent_port_prefix, parent_port_idx_base, vlan_id, nhop_num): + ifnames = [] + parent_port_idx = parent_port_idx_base + for i in range(0, nhop_num): + if vlan_id != 0: + port_name = "{}{}.{}".format(parent_port_prefix, parent_port_idx, vlan_id) + else: + port_name = "{}{}".format(parent_port_prefix, parent_port_idx) + ip_addr = "10.{}.{}.0/31".format(parent_port_idx, vlan_id) + if vlan_id != 0: + self.create_sub_port_intf_profile(port_name) + self.add_sub_port_intf_ip_addr(port_name, ip_addr) + else: + dvs.add_ip_address(port_name, ip_addr) + + ifnames.append(port_name) + + parent_port_idx += (4 if parent_port_prefix == ETHERNET_PREFIX else 1) + return ifnames + + def create_nhg_next_hop_objs(self, dvs, parent_port_prefix, parent_port_idx_base, vlan_id, nhop_num): + nhop_ips = [] + parent_port_idx = parent_port_idx_base + for i in range(0, nhop_num): + nhop_ip = "10.{}.{}.1".format(parent_port_idx, vlan_id) + nhop_mac = "00:00:00:{:02d}:{}:01".format(parent_port_idx, vlan_id) + dvs.runcmd("arp -s " + nhop_ip + " " + nhop_mac) + + nhop_ips.append(nhop_ip) + + parent_port_idx += (4 if parent_port_prefix == ETHERNET_PREFIX else 1) + return nhop_ips + + def remove_nhg_router_intfs(self, dvs, parent_port_prefix, parent_port_idx_base, vlan_id, nhop_num): + parent_port_idx = parent_port_idx_base + for i in range(0, nhop_num): + if vlan_id != 0: + port_name = "{}{}.{}".format(parent_port_prefix, parent_port_idx, vlan_id) + else: + port_name = "{}{}".format(parent_port_prefix, parent_port_idx) + ip_addr = "10.{}.{}.0/31".format(parent_port_idx, vlan_id) + if vlan_id != 0: + # Remove IP address on sub port interface + self.remove_sub_port_intf_ip_addr(port_name, ip_addr) + # Remove sub port interface + self.remove_sub_port_intf_profile(port_name) + else: + dvs.remove_ip_address(port_name, ip_addr) + + parent_port_idx += (4 if parent_port_prefix == ETHERNET_PREFIX else 1) + + def remove_nhg_next_hop_objs(self, dvs, parent_port_prefix, parent_port_idx_base, vlan_id, nhop_num): + parent_port_idx = parent_port_idx_base + for i in range(0, nhop_num): + nhop_ip = "10.{}.{}.1".format(parent_port_idx, vlan_id) + dvs.runcmd("arp -d " + nhop_ip) + + parent_port_idx += (4 if parent_port_prefix == ETHERNET_PREFIX else 1) + + def check_nhg_members_on_parent_port_oper_status_change(self, dvs, parent_port_prefix, parent_port_idx_base, status, \ + nhg_oid, nhop_num, create_intf_on_parent_port): + parent_port_idx = parent_port_idx_base + for i in range(0, nhop_num): + port_name = "{}{}".format(parent_port_prefix, parent_port_idx) + self.set_parent_port_oper_status(dvs, port_name, status) + + # Verify parent port oper status + fv_dict = { + "oper_status" : status, + } + if parent_port_prefix == ETHERNET_PREFIX: + self.check_sub_port_intf_fvs(self.app_db, APP_PORT_TABLE_NAME, port_name, fv_dict) + else: + self.check_sub_port_intf_fvs(self.app_db, APP_LAG_TABLE_NAME, port_name, fv_dict) + + # Verify next hop group member # in ASIC_DB + if status == "up": + nhg_member_oids = self.asic_db.wait_for_n_keys(ASIC_NEXT_HOP_GROUP_MEMBER_TABLE, \ + 1 + i if create_intf_on_parent_port == False else (1 + i) * 2) + else: + assert status == "down" + nhg_member_oids = self.asic_db.wait_for_n_keys(ASIC_NEXT_HOP_GROUP_MEMBER_TABLE, \ + (nhop_num - 1) - i if create_intf_on_parent_port == False else \ + ((nhop_num - 1) - i) * 2) + + # Verify that next hop group members in ASIC_DB all + # belong to the next hop group of the specified oid + fv_dict = { + "SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_GROUP_ID": nhg_oid, + } + for nhg_member_oid in nhg_member_oids: + self.check_sub_port_intf_fvs(self.asic_db, ASIC_NEXT_HOP_GROUP_MEMBER_TABLE, nhg_member_oid, fv_dict) + + parent_port_idx += (4 if parent_port_prefix == ETHERNET_PREFIX else 1) + + def _test_sub_port_intf_nhg_accel(self, dvs, sub_port_intf_name, nhop_num=3, create_intf_on_parent_port=False): + substrs = sub_port_intf_name.split(VLAN_SUB_INTERFACE_SEPARATOR) + parent_port = substrs[0] + vlan_id = substrs[1] + assert len(vlan_id) == 2 + + if parent_port.startswith(ETHERNET_PREFIX): + parent_port_prefix = ETHERNET_PREFIX + else: + assert parent_port.startswith(LAG_PREFIX) + parent_port_prefix = LAG_PREFIX + parent_port_idx_base = self.get_parent_port_index(parent_port) + + # Set parent ports admin status up + parent_port_idx = parent_port_idx_base + for i in range(0, nhop_num): + port_name = "{}{}".format(parent_port_prefix, parent_port_idx) + self.set_parent_port_admin_status(dvs, port_name, "up") + + parent_port_idx += (4 if parent_port_prefix == ETHERNET_PREFIX else 1) + + ifnames = [] + # Create sub port interfaces + ifnames.extend(self.create_nhg_router_intfs(dvs, parent_port_prefix, parent_port_idx_base, int(vlan_id), nhop_num)) + + # Create router interfaces on parent ports + if create_intf_on_parent_port == True: + ifnames.extend(self.create_nhg_router_intfs(dvs, parent_port_prefix, parent_port_idx_base, 0, nhop_num)) + + nhop_ips = [] + nhop_cnt = len(self.asic_db.get_keys(ASIC_NEXT_HOP_TABLE)) + # Create next hop objects on sub port interfaces + nhop_ips.extend(self.create_nhg_next_hop_objs(dvs, parent_port_prefix, parent_port_idx_base, int(vlan_id), nhop_num)) + + # Create next hop objects on router interfaces + if create_intf_on_parent_port == True: + nhop_ips.extend(self.create_nhg_next_hop_objs(dvs, parent_port_prefix, parent_port_idx_base, 0, nhop_num)) + + self.asic_db.wait_for_n_keys(ASIC_NEXT_HOP_TABLE, nhop_cnt + nhop_num if create_intf_on_parent_port == False else nhop_cnt + nhop_num * 2) + + # Create multi-next-hop route entry + rt_tbl = swsscommon.ProducerStateTable(self.app_db.db_connection, APP_ROUTE_TABLE_NAME) + fvs = swsscommon.FieldValuePairs([("nexthop", ",".join(nhop_ips)), ("ifname", ",".join(ifnames))]) + ip_prefix = "2.2.2.0/24" + rt_tbl.set(ip_prefix, fvs) + + # Verify route entry created in ASIC_DB and get next hop group oid + nhg_oid = self.get_ip_prefix_nhg_oid(ip_prefix) + + # Verify next hop group of the specified oid created in ASIC_DB + self.check_sub_port_intf_key_existence(self.asic_db, ASIC_NEXT_HOP_GROUP_TABLE, nhg_oid) + + # Verify next hop group members created in ASIC_DB + nhg_member_oids = self.asic_db.wait_for_n_keys(ASIC_NEXT_HOP_GROUP_MEMBER_TABLE, \ + nhop_num if create_intf_on_parent_port == False else nhop_num * 2) + + # Verify that next hop group members all belong to the next hop group of the specified oid + fv_dict = { + "SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_GROUP_ID": nhg_oid, + } + for nhg_member_oid in nhg_member_oids: + self.check_sub_port_intf_fvs(self.asic_db, ASIC_NEXT_HOP_GROUP_MEMBER_TABLE, nhg_member_oid, fv_dict) + + # Bring parent ports oper status down one at a time, and verify next hop group members + self.check_nhg_members_on_parent_port_oper_status_change(dvs, parent_port_prefix, parent_port_idx_base, "down", \ + nhg_oid, nhop_num, create_intf_on_parent_port) + + # Bring parent ports oper status up one at a time, and verify next hop group members + self.check_nhg_members_on_parent_port_oper_status_change(dvs, parent_port_prefix, parent_port_idx_base, "up", \ + nhg_oid, nhop_num, create_intf_on_parent_port) + + # Clean up + rif_cnt = len(self.asic_db.get_keys(ASIC_RIF_TABLE)) + + # Remove sub port interfaces + self.remove_nhg_router_intfs(dvs, parent_port_prefix, parent_port_idx_base, int(vlan_id), nhop_num) + + # Remove router interfaces on parent ports + if create_intf_on_parent_port == True: + self.remove_nhg_router_intfs(dvs, parent_port_prefix, parent_port_idx_base, 0, nhop_num) + + # Remove ecmp route entry + rt_tbl._del(ip_prefix) + + # Removal of router interfaces indicates the proper removal of nhg, nhg members, next hop objects, and neighbor entries + self.asic_db.wait_for_n_keys(ASIC_RIF_TABLE, rif_cnt - nhop_num if create_intf_on_parent_port == False else rif_cnt - nhop_num * 2) + + # Make sure parent port is oper status up + parent_port_idx = parent_port_idx_base + for i in range(0, nhop_num): + port_name = "{}{}".format(parent_port_prefix, parent_port_idx) + self.set_parent_port_oper_status(dvs, port_name, "up") + + parent_port_idx += (4 if parent_port_prefix == ETHERNET_PREFIX else 1) + + def test_sub_port_intf_nhg_accel(self, dvs): + self.connect_dbs(dvs) + + self._test_sub_port_intf_nhg_accel(dvs, self.SUB_PORT_INTERFACE_UNDER_TEST) + self._test_sub_port_intf_nhg_accel(dvs, self.SUB_PORT_INTERFACE_UNDER_TEST, create_intf_on_parent_port=True) + self._test_sub_port_intf_nhg_accel(dvs, self.LAG_SUB_PORT_INTERFACE_UNDER_TEST) + self._test_sub_port_intf_nhg_accel(dvs, self.LAG_SUB_PORT_INTERFACE_UNDER_TEST, create_intf_on_parent_port=True) + + def _test_sub_port_intf_oper_down_with_pending_neigh_route_tasks(self, dvs, sub_port_intf_name, nhop_num=3, create_intf_on_parent_port=False): + substrs = sub_port_intf_name.split(VLAN_SUB_INTERFACE_SEPARATOR) + parent_port = substrs[0] + vlan_id = substrs[1] + assert len(vlan_id) == 2 + + if parent_port.startswith(ETHERNET_PREFIX): + parent_port_prefix = ETHERNET_PREFIX + else: + assert parent_port.startswith(LAG_PREFIX) + parent_port_prefix = LAG_PREFIX + parent_port_idx_base = self.get_parent_port_index(parent_port) + + # Set parent ports admin status up + parent_port_idx = parent_port_idx_base + for i in range(0, nhop_num): + port_name = "{}{}".format(parent_port_prefix, parent_port_idx) + self.set_parent_port_admin_status(dvs, port_name, "up") + + parent_port_idx += (4 if parent_port_prefix == ETHERNET_PREFIX else 1) + + ifnames = [] + # Create sub port interfaces + ifnames.extend(self.create_nhg_router_intfs(dvs, parent_port_prefix, parent_port_idx_base, int(vlan_id), nhop_num)) + # Create router interfaces on parent ports + if create_intf_on_parent_port == True: + ifnames.extend(self.create_nhg_router_intfs(dvs, parent_port_prefix, parent_port_idx_base, 0, nhop_num)) + + # Bring parent port oper status down one at a time + # Verify next hop group members created after processing pending tasks + parent_port_idx = parent_port_idx_base + for i in range(0, nhop_num): + port_name = "{}{}".format(parent_port_prefix, parent_port_idx) + self.set_parent_port_oper_status(dvs, port_name, "down") + + # Verify parent port oper status down + fv_dict = { + "oper_status" : "down", + } + if parent_port_prefix == ETHERNET_PREFIX: + self.check_sub_port_intf_fvs(self.app_db, APP_PORT_TABLE_NAME, port_name, fv_dict) + else: + self.check_sub_port_intf_fvs(self.app_db, APP_LAG_TABLE_NAME, port_name, fv_dict) + + # Mimic pending neighbor task + nhop_ips = [] + nhop_cnt = len(self.asic_db.get_keys(ASIC_NEXT_HOP_TABLE)) + # Create next hop objects on sub port interfaces + nhop_ips.extend(self.create_nhg_next_hop_objs(dvs, parent_port_prefix, parent_port_idx_base, int(vlan_id), nhop_num)) + # Create next hop objects on router interfaces + if create_intf_on_parent_port == True: + nhop_ips.extend(self.create_nhg_next_hop_objs(dvs, parent_port_prefix, parent_port_idx_base, 0, nhop_num)) + self.asic_db.wait_for_n_keys(ASIC_NEXT_HOP_TABLE, nhop_cnt + nhop_num if create_intf_on_parent_port == False else nhop_cnt + nhop_num * 2) + + # Mimic pending multi-next-hop route entry task + rt_tbl = swsscommon.ProducerStateTable(self.app_db.db_connection, APP_ROUTE_TABLE_NAME) + fvs = swsscommon.FieldValuePairs([("nexthop", ",".join(nhop_ips)), ("ifname", ",".join(ifnames))]) + ip_prefix = "2.2.2.0/24" + rt_tbl.set(ip_prefix, fvs) + + # Verify route entry created in ASIC_DB and get next hop group oid + nhg_oid = self.get_ip_prefix_nhg_oid(ip_prefix) + + # Verify next hop group of the specified oid created in ASIC_DB + self.check_sub_port_intf_key_existence(self.asic_db, ASIC_NEXT_HOP_GROUP_TABLE, nhg_oid) + + # Verify next hop group member # created in ASIC_DB + nhg_member_oids = self.asic_db.wait_for_n_keys(ASIC_NEXT_HOP_GROUP_MEMBER_TABLE, \ + (nhop_num - 1) - i if create_intf_on_parent_port == False else ((nhop_num - 1) - i) * 2) + + # Verify that next hop group members all belong to the next hop group of the specified oid + fv_dict = { + "SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_GROUP_ID": nhg_oid, + } + for nhg_member_oid in nhg_member_oids: + self.check_sub_port_intf_fvs(self.asic_db, ASIC_NEXT_HOP_GROUP_MEMBER_TABLE, nhg_member_oid, fv_dict) + + nhop_cnt = len(self.asic_db.get_keys(ASIC_NEXT_HOP_TABLE)) + # Remove next hop objects on sub port interfaces + self.remove_nhg_next_hop_objs(dvs, parent_port_prefix, parent_port_idx_base, int(vlan_id), nhop_num) + # Remove next hop objects on router interfaces + if create_intf_on_parent_port == True: + self.remove_nhg_next_hop_objs(dvs, parent_port_prefix, parent_port_idx_base, 0, nhop_num) + # Remove ecmp route entry + rt_tbl._del(ip_prefix) + # Removal of next hop objects indicates the proper removal of route entry, nhg, and nhg members + self.asic_db.wait_for_n_keys(ASIC_NEXT_HOP_TABLE, nhop_cnt - nhop_num if create_intf_on_parent_port == False else nhop_cnt - nhop_num * 2) + + parent_port_idx += (4 if parent_port_prefix == ETHERNET_PREFIX else 1) + + # Clean up + rif_cnt = len(self.asic_db.get_keys(ASIC_RIF_TABLE)) + # Remove sub port interfaces + self.remove_nhg_router_intfs(dvs, parent_port_prefix, parent_port_idx_base, int(vlan_id), nhop_num) + # Remove router interfaces on parent ports + if create_intf_on_parent_port == True: + self.remove_nhg_router_intfs(dvs, parent_port_prefix, parent_port_idx_base, 0, nhop_num) + self.asic_db.wait_for_n_keys(ASIC_RIF_TABLE, rif_cnt - nhop_num if create_intf_on_parent_port == False else rif_cnt - nhop_num * 2) + + # Make sure parent port oper status is up + parent_port_idx = parent_port_idx_base + for i in range(0, nhop_num): + port_name = "{}{}".format(parent_port_prefix, parent_port_idx) + self.set_parent_port_oper_status(dvs, port_name, "up") + + parent_port_idx += (4 if parent_port_prefix == ETHERNET_PREFIX else 1) + + def test_sub_port_intf_oper_down_with_pending_neigh_route_tasks(self, dvs): + self.connect_dbs(dvs) + + self._test_sub_port_intf_oper_down_with_pending_neigh_route_tasks(dvs, self.SUB_PORT_INTERFACE_UNDER_TEST) + self._test_sub_port_intf_oper_down_with_pending_neigh_route_tasks(dvs, self.SUB_PORT_INTERFACE_UNDER_TEST, create_intf_on_parent_port=True) + self._test_sub_port_intf_oper_down_with_pending_neigh_route_tasks(dvs, self.LAG_SUB_PORT_INTERFACE_UNDER_TEST) + self._test_sub_port_intf_oper_down_with_pending_neigh_route_tasks(dvs, self.LAG_SUB_PORT_INTERFACE_UNDER_TEST, create_intf_on_parent_port=True) + # Add Dummy always-pass test at end as workaroud # for issue when Flaky fail on final test it invokes module tear-down before retrying From 6345d4d07899f8bb9b74d17bb01408cfd58de99c Mon Sep 17 00:00:00 2001 From: Wenda Ni Date: Sun, 1 Nov 2020 21:50:18 +0000 Subject: [PATCH 4/7] Remove redundant string Signed-off-by: Wenda Ni --- tests/test_sub_port_intf.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_sub_port_intf.py b/tests/test_sub_port_intf.py index c7afe2de81..ec1b88cb8c 100644 --- a/tests/test_sub_port_intf.py +++ b/tests/test_sub_port_intf.py @@ -25,7 +25,6 @@ ASIC_NEXT_HOP_TABLE = "ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP" ASIC_NEXT_HOP_GROUP_TABLE = "ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP" ASIC_NEXT_HOP_GROUP_MEMBER_TABLE = "ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER" -ASIC_NEXT_HOP_TABLE = "ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP" ADMIN_STATUS = "admin_status" From 5851e363c39f75ff58e75550fe805dcbcff0a034 Mon Sep 17 00:00:00 2001 From: Wenda Ni Date: Sun, 15 Nov 2020 00:05:16 +0000 Subject: [PATCH 5/7] Address comment: refactor code to reuse updatePortOperStatus() Signed-off-by: Wenda Ni --- orchagent/portsorch.cpp | 40 ++++++++-------------------------------- 1 file changed, 8 insertions(+), 32 deletions(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 3dc930e5dd..85ac2a3871 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -2815,7 +2815,6 @@ void PortsOrch::doLagTask(Consumer &consumer) // Retrieve attributes uint32_t mtu = 0; string learn_mode; - bool operation_status_changed = false; string operation_status; for (auto i : kfvFieldsValues(t)) @@ -2837,14 +2836,6 @@ void PortsOrch::doLagTask(Consumer &consumer) it++; continue; } - - Port lag; - if (getPort(alias, lag)) - { - operation_status_changed = - (string_oper_status.at(operation_status) != - lag.m_oper_status); - } } } @@ -2868,28 +2859,9 @@ void PortsOrch::doLagTask(Consumer &consumer) { if (!operation_status.empty()) { - l.m_oper_status = string_oper_status.at(operation_status); - m_portList[alias] = l; + updatePortOperStatus(l, string_oper_status.at(operation_status)); - bool isUp = operation_status == "up" ? true : false; - if (!gNeighOrch->ifChangeInformNextHop(alias, isUp)) - { - SWSS_LOG_WARN("Inform nexthop operation failed for interface %s", alias.c_str()); - } - for (const auto &child_port : l.m_child_ports) - { - if (!gNeighOrch->ifChangeInformNextHop(child_port, isUp)) - { - SWSS_LOG_WARN("Inform nexthop operation failed for sub interface %s", child_port.c_str()); - } - } - } - if (operation_status_changed) - { - PortOperStateUpdate update; - update.port = l; - update.operStatus = string_oper_status.at(operation_status); - notify(SUBJECT_TYPE_PORT_OPER_STATE_CHANGE, static_cast(&update)); + m_portList[alias] = l; } if (mtu != 0) @@ -4138,11 +4110,15 @@ void PortsOrch::updatePortOperStatus(Port &port, sai_port_oper_status_t status) return ; } - updateDbPortOperStatus(port, status); + if (port.m_type == Port::PHY) + { + updateDbPortOperStatus(port, status); + } port.m_oper_status = status; bool isUp = status == SAI_PORT_OPER_STATUS_UP; - if (!setHostIntfsOperStatus(port, isUp)) + // Host intf is a notion for physical port only + if (port.m_type == Port::PHY && !setHostIntfsOperStatus(port, isUp)) { SWSS_LOG_ERROR("Failed to set host interface %s operational status %s", port.m_alias.c_str(), isUp ? "up" : "down"); From c062f946886fcd7354035f703ea28c8d6f395a59 Mon Sep 17 00:00:00 2001 From: Wenda Ni Date: Sun, 15 Nov 2020 00:08:23 +0000 Subject: [PATCH 6/7] Refactor Signed-off-by: Wenda Ni --- orchagent/portsorch.cpp | 2 +- orchagent/portsorch.h | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 85ac2a3871..ecb3a4e60f 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -4107,7 +4107,7 @@ void PortsOrch::updatePortOperStatus(Port &port, sai_port_oper_status_t status) oper_status_strings.at(status).c_str()); if (status == port.m_oper_status) { - return ; + return; } if (port.m_type == Port::PHY) diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index 4d7ebb25b1..cff208399e 100755 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -37,10 +37,10 @@ static const map oper_status_strings = static const unordered_map string_oper_status = { - { "unknown", SAI_PORT_OPER_STATUS_UNKNOWN }, - { "up", SAI_PORT_OPER_STATUS_UP }, - { "down", SAI_PORT_OPER_STATUS_DOWN }, - { "testing", SAI_PORT_OPER_STATUS_TESTING }, + { "unknown", SAI_PORT_OPER_STATUS_UNKNOWN }, + { "up", SAI_PORT_OPER_STATUS_UP }, + { "down", SAI_PORT_OPER_STATUS_DOWN }, + { "testing", SAI_PORT_OPER_STATUS_TESTING }, { "not present", SAI_PORT_OPER_STATUS_NOT_PRESENT } }; From f5f8c4562036e82609098203e60dc73b8090bdba Mon Sep 17 00:00:00 2001 From: Wenda Ni Date: Sun, 15 Nov 2020 00:30:42 +0000 Subject: [PATCH 7/7] Refactor Signed-off-by: Wenda Ni --- orchagent/portsorch.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index ecb3a4e60f..a54db9bacc 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -4117,11 +4117,13 @@ void PortsOrch::updatePortOperStatus(Port &port, sai_port_oper_status_t status) port.m_oper_status = status; bool isUp = status == SAI_PORT_OPER_STATUS_UP; - // Host intf is a notion for physical port only - if (port.m_type == Port::PHY && !setHostIntfsOperStatus(port, isUp)) + if (port.m_type == Port::PHY) { - SWSS_LOG_ERROR("Failed to set host interface %s operational status %s", port.m_alias.c_str(), - isUp ? "up" : "down"); + if (!setHostIntfsOperStatus(port, isUp)) + { + SWSS_LOG_ERROR("Failed to set host interface %s operational status %s", port.m_alias.c_str(), + isUp ? "up" : "down"); + } } if (!gNeighOrch->ifChangeInformNextHop(port.m_alias, isUp)) {