From 2c8da6ba421073123174649afe46e5e61b99035a Mon Sep 17 00:00:00 2001 From: Jaganathan Anbalagan Date: Thu, 12 May 2022 16:52:59 -0700 Subject: [PATCH 1/6] Signed-off-by: Jaganathan Anbalagan orchagent changes for https://github.com/sonic-net/SONiC/pull/916 --- orchagent/portsorch.cpp | 61 +++++++++++++++++++++++++++++++++++++---- orchagent/portsorch.h | 1 + 2 files changed, 57 insertions(+), 5 deletions(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 687d1e915a..a528b4cdd4 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -1032,6 +1032,36 @@ void PortsOrch::getCpuPort(Port &port) port = m_cpuPort; } +bool PortsOrch::initHostTxReadyState(Port &port) +{ + SWSS_LOG_ENTER(); + + vector tuples; + bool exist = m_portStateTable.get(port.m_alias, tuples); + string hostTxReady; + + if (exist) + { + for (auto i : tuples) + { + if (fvField(i) == "host_tx_ready") + { + hostTxReady = fvValue(i); + } + } + } + + /* Create host_tx_ready field in state-DB and set it to false by default. */ + if (hostTxReady.empty()) + { + m_portStateTable.hset(port.m_alias, "host_tx_ready", "false"); + SWSS_LOG_INFO("initalize hostTxReady %s with status %s", + port.m_alias.c_str(), hostTxReady.c_str()); + } + + return true; +} + bool PortsOrch::setPortAdminStatus(Port &port, bool state) { SWSS_LOG_ENTER(); @@ -1040,6 +1070,13 @@ bool PortsOrch::setPortAdminStatus(Port &port, bool state) attr.id = SAI_PORT_ATTR_ADMIN_STATE; attr.value.booldata = state; + /* Update the host_tx_ready to false before setting admin_state, when admin state is false */ + if (!state) + { + m_portStateTable.hset(port.m_alias, "host_tx_ready", "false"); + SWSS_LOG_INFO("Set admin status false %s to port pid:%" PRIx64, state ? "true" : "false", port.m_port_id); + } + sai_status_t status = sai_port_api->set_port_attribute(port.m_port_id, &attr); if (status != SAI_STATUS_SUCCESS) { @@ -1052,10 +1089,20 @@ bool PortsOrch::setPortAdminStatus(Port &port, bool state) } } - SWSS_LOG_INFO("Set admin status %s to port pid:%" PRIx64, - state ? "UP" : "DOWN", port.m_port_id); - - setGearboxPortsAttr(port, SAI_PORT_ATTR_ADMIN_STATE, &state); + bool gbstatus = setGearboxPortsAttr(port, SAI_PORT_ATTR_ADMIN_STATE, &state); + if(gbstatus != true) + { + SWSS_LOG_ERROR("Failed to set admin status on PHY %s to port pid:%" PRIx64, + state ? "UP" : "DOWN", port.m_port_id); + /* add task handler to recover/take action on this failure */ + } + + /* Update the state table for 'host_tx_ready'*/ + if (state && (gbstatus == true) && (status == SAI_STATUS_SUCCESS) ) + { + m_portStateTable.hset(port.m_alias, "host_tx_ready", "true"); + SWSS_LOG_INFO("Set admin status true %s to port pid:%" PRIx64, state ? "true" : "false", port.m_port_id); + } return true; } @@ -1940,7 +1987,7 @@ void PortsOrch::initPortSupportedSpeeds(const std::string& alias, sai_object_id_ */ bool PortsOrch::setGearboxPortsAttr(Port &port, sai_port_attr_t id, void *value) { - bool status; + bool status = false; status = setGearboxPortAttr(port, PHY_PORT_TYPE, id, value); @@ -3368,6 +3415,10 @@ void PortsOrch::doPortTask(Consumer &consumer) } + if (!initHostTxReadyState(p)) { + SWSS_LOG_ERROR("Failed to initialize host_tx_ready for port %s ", alias.c_str()); + } + /* Last step set port admin status */ if (!admin_status.empty() && (p.m_admin_state_up != (admin_status == "up"))) { diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index c820d6969d..ad498f4bc2 100755 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -95,6 +95,7 @@ class PortsOrch : public Orch, public Subject bool getPortByBridgePortId(sai_object_id_t bridge_port_id, Port &port); void setPort(string alias, Port port); void getCpuPort(Port &port); + bool initHostTxReadyState(Port &port); bool getInbandPort(Port &port); bool getVlanByVlanId(sai_vlan_id_t vlan_id, Port &vlan); From ed537d814f66bc2c8c74cd771bc652662dafce45 Mon Sep 17 00:00:00 2001 From: Jaganathan Anbalagan Date: Fri, 20 May 2022 07:50:58 -0700 Subject: [PATCH 2/6] Signed-off-by: Jaganathan Anbalagan Addressing PR comment --- orchagent/portsorch.cpp | 15 ++++++--------- orchagent/portsorch.h | 2 +- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index a528b4cdd4..99a22f595f 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -1032,7 +1032,7 @@ void PortsOrch::getCpuPort(Port &port) port = m_cpuPort; } -bool PortsOrch::initHostTxReadyState(Port &port) +void PortsOrch::initHostTxReadyState(Port &port) { SWSS_LOG_ENTER(); @@ -1058,8 +1058,6 @@ bool PortsOrch::initHostTxReadyState(Port &port) SWSS_LOG_INFO("initalize hostTxReady %s with status %s", port.m_alias.c_str(), hostTxReady.c_str()); } - - return true; } bool PortsOrch::setPortAdminStatus(Port &port, bool state) @@ -1090,7 +1088,7 @@ bool PortsOrch::setPortAdminStatus(Port &port, bool state) } bool gbstatus = setGearboxPortsAttr(port, SAI_PORT_ATTR_ADMIN_STATE, &state); - if(gbstatus != true) + if (gbstatus != true) { SWSS_LOG_ERROR("Failed to set admin status on PHY %s to port pid:%" PRIx64, state ? "UP" : "DOWN", port.m_port_id); @@ -3414,11 +3412,10 @@ void PortsOrch::doPortTask(Consumer &consumer) } } - - if (!initHostTxReadyState(p)) { - SWSS_LOG_ERROR("Failed to initialize host_tx_ready for port %s ", alias.c_str()); - } - + + /* create host_tx_ready field in state-db */ + initHostTxReadyState(p) + /* Last step set port admin status */ if (!admin_status.empty() && (p.m_admin_state_up != (admin_status == "up"))) { diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index ad498f4bc2..0fd3552e19 100755 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -95,7 +95,7 @@ class PortsOrch : public Orch, public Subject bool getPortByBridgePortId(sai_object_id_t bridge_port_id, Port &port); void setPort(string alias, Port port); void getCpuPort(Port &port); - bool initHostTxReadyState(Port &port); + void initHostTxReadyState(Port &port); bool getInbandPort(Port &port); bool getVlanByVlanId(sai_vlan_id_t vlan_id, Port &vlan); From b118f9ec9a7a0062637d812e8994e802231dacbb Mon Sep 17 00:00:00 2001 From: Jaganathan Anbalagan Date: Thu, 2 Jun 2022 15:00:10 -0700 Subject: [PATCH 3/6] Signed-off-by: Jaganathan Anbalagan Addressing PR comments-cosmetic --- orchagent/portsorch.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 99a22f595f..906e28362b 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -1032,6 +1032,11 @@ void PortsOrch::getCpuPort(Port &port) port = m_cpuPort; } +/* + * Create host_tx_ready field in PORT_TABLE of STATE-DB + * and set the field to false by default for the + * front port. + */ void PortsOrch::initHostTxReadyState(Port &port) { SWSS_LOG_ENTER(); @@ -1051,7 +1056,6 @@ void PortsOrch::initHostTxReadyState(Port &port) } } - /* Create host_tx_ready field in state-DB and set it to false by default. */ if (hostTxReady.empty()) { m_portStateTable.hset(port.m_alias, "host_tx_ready", "false"); @@ -1092,7 +1096,7 @@ bool PortsOrch::setPortAdminStatus(Port &port, bool state) { SWSS_LOG_ERROR("Failed to set admin status on PHY %s to port pid:%" PRIx64, state ? "UP" : "DOWN", port.m_port_id); - /* add task handler to recover/take action on this failure */ + /* TODO add task handler to recover/take action on this failure */ } /* Update the state table for 'host_tx_ready'*/ From de9928af1951a312270e491762a2186c8bd7e015 Mon Sep 17 00:00:00 2001 From: Jaganathan Anbalagan Date: Fri, 24 Jun 2022 09:43:12 -0700 Subject: [PATCH 4/6] Signed-off-by: Jaganathan Anbalagan fixed typo --- orchagent/portsorch.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 906e28362b..444ecd4051 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -3418,7 +3418,7 @@ void PortsOrch::doPortTask(Consumer &consumer) } /* create host_tx_ready field in state-db */ - initHostTxReadyState(p) + initHostTxReadyState(p); /* Last step set port admin status */ if (!admin_status.empty() && (p.m_admin_state_up != (admin_status == "up"))) From f285b735c2abdd09d6f18b0443358dbce23cdaf1 Mon Sep 17 00:00:00 2001 From: Jaganathan Anbalagan Date: Wed, 29 Jun 2022 15:09:31 -0700 Subject: [PATCH 5/6] Signed-off-by: Jaganathan Anbalagan VS test code and addressing PR comment --- orchagent/portsorch.cpp | 21 ++++++++++++--------- tests/test_admin_status.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 444ecd4051..028d544c04 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -1076,7 +1076,8 @@ bool PortsOrch::setPortAdminStatus(Port &port, bool state) if (!state) { m_portStateTable.hset(port.m_alias, "host_tx_ready", "false"); - SWSS_LOG_INFO("Set admin status false %s to port pid:%" PRIx64, state ? "true" : "false", port.m_port_id); + SWSS_LOG_INFO("Set admin status DOWN host_tx_ready to false to port pid:%" PRIx64, + port.m_port_id); } sai_status_t status = sai_port_api->set_port_attribute(port.m_port_id, &attr); @@ -1084,6 +1085,7 @@ bool PortsOrch::setPortAdminStatus(Port &port, bool state) { SWSS_LOG_ERROR("Failed to set admin status %s to port pid:%" PRIx64, state ? "UP" : "DOWN", port.m_port_id); + m_portStateTable.hset(port.m_alias, "host_tx_ready", "false"); task_process_status handle_status = handleSaiSetStatus(SAI_API_PORT, status); if (handle_status != task_success) { @@ -1092,18 +1094,19 @@ bool PortsOrch::setPortAdminStatus(Port &port, bool state) } bool gbstatus = setGearboxPortsAttr(port, SAI_PORT_ATTR_ADMIN_STATE, &state); - if (gbstatus != true) - { - SWSS_LOG_ERROR("Failed to set admin status on PHY %s to port pid:%" PRIx64, - state ? "UP" : "DOWN", port.m_port_id); - /* TODO add task handler to recover/take action on this failure */ - } - /* Update the state table for 'host_tx_ready'*/ + /* Update the state table for host_tx_ready*/ if (state && (gbstatus == true) && (status == SAI_STATUS_SUCCESS) ) { m_portStateTable.hset(port.m_alias, "host_tx_ready", "true"); - SWSS_LOG_INFO("Set admin status true %s to port pid:%" PRIx64, state ? "true" : "false", port.m_port_id); + SWSS_LOG_INFO("Set admin status UP host_tx_ready to true to port pid:%" PRIx64, + port.m_port_id); + } + else + { + m_portStateTable.hset(port.m_alias, "host_tx_ready", "false"); + SWSS_LOG_INFO("Set admin status %s host_tx_ready to false to port pid:%" PRIx64, + state ? "UP" : "DOWN", port.m_port_id); } return true; diff --git a/tests/test_admin_status.py b/tests/test_admin_status.py index 15724a7c02..1b99bf37c7 100644 --- a/tests/test_admin_status.py +++ b/tests/test_admin_status.py @@ -9,6 +9,7 @@ def setup_db(self, dvs): self.pdb = swsscommon.DBConnector(0, dvs.redis_sock, 0) self.adb = swsscommon.DBConnector(1, dvs.redis_sock, 0) self.cdb = swsscommon.DBConnector(4, dvs.redis_sock, 0) + self.sdb = swsscommon.DBConnector(6, dvs.redis_sock, 0) def set_admin_status(self, port, admin_status): assert admin_status == "up" or admin_status == "down" @@ -52,6 +53,16 @@ def check_admin_status(self, dvs, port, admin_status): if fv[0] == "SAI_PORT_ATTR_ADMIN_STATE": assert fv[1] == "true" if admin_status == "up" else "false" + def check_host_tx_ready_status(self, dvs, port, admin_status): + assert admin_status == "up" or admin_status == "down" + ptbl = swsscommon.Table(self.sdb, "PORT_TABLE") + (status, fvs) = ptbl.get(port) + assert status == True + assert "host_tx_ready" in [fv[0] for fv in fvs] + for fv in fvs: + if fv[0] == "host_tx_ready": + assert fv[1] == "true" if admin_status == "up" else "false" + def test_PortChannelMemberAdminStatus(self, dvs, testlog): self.setup_db(dvs) @@ -79,6 +90,24 @@ def test_PortChannelMemberAdminStatus(self, dvs, testlog): # remove port channel self.remove_port_channel(dvs, "PortChannel6") + def test_PortHostTxReadiness(self, dvs, testlog): + self.setup_db(dvs) + + # configure admin status to interface + self.set_admin_status("Ethernet0", "up") + self.set_admin_status("Ethernet4", "down") + self.set_admin_status("Ethernet8", "up") + + # check ASIC port database + self.check_admin_status(dvs, "Ethernet0", "up") + self.check_admin_status(dvs, "Ethernet4", "down") + self.check_admin_status(dvs, "Ethernet8", "up") + + # check host readiness status in PORT TABLE of STATE-DB + self.check_host_tx_ready_status(dvs, "Ethernet0", "up") + self.check_host_tx_ready_status(dvs, "Ethernet4", "down") + self.check_host_tx_ready_status(dvs, "Ethernet8", "up") + # 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 f6122a9f5fbb1e54fe5db3834a28fe7d4a8f32bc Mon Sep 17 00:00:00 2001 From: Jaganathan Anbalagan Date: Wed, 29 Jun 2022 17:02:26 -0700 Subject: [PATCH 6/6] Signed-off-by: Jaganathan Anbalagan set host_tx_ready to false if gbsyncd SAI API fails. --- orchagent/portsorch.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 028d544c04..181e7d4e4b 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -1094,6 +1094,10 @@ bool PortsOrch::setPortAdminStatus(Port &port, bool state) } bool gbstatus = setGearboxPortsAttr(port, SAI_PORT_ATTR_ADMIN_STATE, &state); + if (gbstatus != true) + { + m_portStateTable.hset(port.m_alias, "host_tx_ready", "false"); + } /* Update the state table for host_tx_ready*/ if (state && (gbstatus == true) && (status == SAI_STATUS_SUCCESS) ) @@ -1102,12 +1106,6 @@ bool PortsOrch::setPortAdminStatus(Port &port, bool state) SWSS_LOG_INFO("Set admin status UP host_tx_ready to true to port pid:%" PRIx64, port.m_port_id); } - else - { - m_portStateTable.hset(port.m_alias, "host_tx_ready", "false"); - SWSS_LOG_INFO("Set admin status %s host_tx_ready to false to port pid:%" PRIx64, - state ? "UP" : "DOWN", port.m_port_id); - } return true; }