From b0bee3ec7bd1c00c976e812eae27f0e88d41f630 Mon Sep 17 00:00:00 2001 From: Dante Su Date: Thu, 14 Apr 2022 07:54:14 +0000 Subject: [PATCH] address review comments Signed-off-by: Dante Su --- orchagent/port.h | 1 + orchagent/portsorch.cpp | 80 ++++++++++++++++++++++++++++------------- orchagent/portsorch.h | 15 +++++--- tests/test_port_lt.py | 4 +-- 4 files changed, 70 insertions(+), 30 deletions(-) diff --git a/orchagent/port.h b/orchagent/port.h index cbdc8fa9d74..ee0320cc26a 100644 --- a/orchagent/port.h +++ b/orchagent/port.h @@ -176,6 +176,7 @@ class Port bool m_fec_cfg = false; bool m_an_cfg = false; + bool m_port_cap_lt = false; /* Port Capability - LinkTraining */ }; } diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index ba58fac07b5..9f8071a42dd 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -325,7 +325,6 @@ static bool isValidPortTypeForLagMember(const Port& port) PortsOrch::PortsOrch(DBConnector *db, DBConnector *stateDb, vector &tableNames, DBConnector *chassisAppDb) : Orch(db, tableNames), m_portStateTable(stateDb, STATE_PORT_TABLE_NAME), - m_stateLinkTrainingTable(stateDb, STATE_LINK_TRAINING_TABLE_NAME), port_stat_manager(PORT_STAT_COUNTER_FLEX_COUNTER_GROUP, StatsMode::READ, PORT_STAT_FLEX_COUNTER_POLLING_INTERVAL_MS, false), port_buffer_drop_stat_manager(PORT_BUFFER_DROP_STAT_FLEX_COUNTER_GROUP, StatsMode::READ, PORT_BUFFER_DROP_STAT_POLLING_INTERVAL_MS, false), queue_stat_manager(QUEUE_STAT_COUNTER_FLEX_COUNTER_GROUP, StatsMode::READ, QUEUE_STAT_FLEX_COUNTER_POLLING_INTERVAL_MS, false), @@ -1932,7 +1931,7 @@ void PortsOrch::initPortSupportedSpeeds(const std::string& alias, sai_object_id_ m_portStateTable.set(alias, v); } -void PortsOrch::initPortCapabilityLinkTraining(const Port &port) +void PortsOrch::initPortCapLinkTraining(Port &port) { sai_status_t status; sai_attribute_t attr; @@ -1941,11 +1940,11 @@ void PortsOrch::initPortCapabilityLinkTraining(const Port &port) status = sai_port_api->set_port_attribute(port.m_port_id, &attr); if (status == SAI_STATUS_SUCCESS) { - string supp = attr.value.booldata ? "yes" : "no"; - m_stateLinkTrainingTable.hset(port.m_alias, "supported", supp); + port.m_port_cap_lt = attr.value.booldata; } else { + port.m_port_cap_lt = true; /* This is for vstest */ SWSS_LOG_NOTICE("Unable to get %s LT capability", port.m_alias.c_str()); } } @@ -2218,17 +2217,6 @@ task_process_status PortsOrch::setPortAutoNeg(sai_object_id_t id, int an) return task_success; } -bool PortsOrch::getPortCapabilityLinkTraining(const Port& port) -{ - string supp; - - if (m_stateLinkTrainingTable.hget(port.m_alias, "supported", supp)) - { - return (supp == "yes") ? true : false; - } - return false; -} - task_process_status PortsOrch::setPortLinkTraining(const Port &port, bool state) { SWSS_LOG_ENTER(); @@ -2480,7 +2468,7 @@ bool PortsOrch::initPort(const string &alias, const string &role, const int inde initGearboxPort(p); /* Initialize port capabilities */ - initPortCapabilityLinkTraining(p); + initPortCapLinkTraining(p); /* Add port to port list */ m_portList[alias] = p; @@ -3081,7 +3069,7 @@ void PortsOrch::doPortTask(Consumer &consumer) lt = link_training_mode_map[lt_str]; if (lt != p.m_link_training) { - if (!getPortCapabilityLinkTraining(p)) + if (!p.m_port_cap_lt) { SWSS_LOG_ERROR("%s: LT is not supported by the ASIC", alias.c_str()); // Don't retry @@ -3103,10 +3091,11 @@ void PortsOrch::doPortTask(Consumer &consumer) } continue; } - m_stateLinkTrainingTable.hset(p.m_alias, "status", lt > 0 ? "on" : "off"); + m_portStateTable.hset(alias, "link_training_status", lt_str); SWSS_LOG_NOTICE("Set port %s LT from %d to %d", alias.c_str(), p.m_link_training, lt); p.m_link_training = lt; m_portList[alias] = p; + updatePortStatePoll(p, PORT_STATE_POLL_LT, (lt > 0)); } } @@ -5938,6 +5927,19 @@ void PortsOrch::updatePortOperStatus(Port &port, sai_port_oper_status_t status) if (port.m_type == Port::PHY) { updateDbPortOperStatus(port, status); + if (port.m_admin_state_up && port.m_link_training > 0) + { + updatePortStateLinkTraining(port); + if (status == SAI_PORT_OPER_STATUS_UP) + { + updatePortStatePoll(port, PORT_STATE_POLL_LT, false); + } + else + { + /* Restart port state polling upon link down event */ + updatePortStatePoll(port, PORT_STATE_POLL_LT, true); + } + } } port.m_oper_status = status; @@ -7127,7 +7129,7 @@ bool PortsOrch::decrFdbCount(const std::string& alias, int count) return true; } -void PortsOrch::updatePortLinkTrainingStatus(const Port &port) +void PortsOrch::updatePortStateLinkTraining(const Port &port) { SWSS_LOG_ENTER(); @@ -7138,7 +7140,7 @@ void PortsOrch::updatePortLinkTrainingStatus(const Port &port) string status = "off"; - if ((port.m_link_training > 0) && getPortCapabilityLinkTraining(port)) + if ((port.m_link_training > 0) && port.m_port_cap_lt) { sai_port_link_training_rx_status_t rx_status; sai_port_link_training_failure_status_t failure; @@ -7164,15 +7166,45 @@ void PortsOrch::updatePortLinkTrainingStatus(const Port &port) } } } - m_stateLinkTrainingTable.hset(port.m_alias, "status", status); + + m_portStateTable.hset(port.m_alias, "link_training_status", status); +} + +void PortsOrch::updatePortStatePoll(const Port &port, port_state_poll_t type, bool set) +{ + if (type == PORT_STATE_POLL_NONE) + { + return; + } + if (set) + { + m_port_state_poll[port.m_alias] |= type; + m_timer->start(); + } + else + { + m_port_state_poll[port.m_alias] &= ~type; + } } void PortsOrch::doTask(swss::SelectableTimer &timer) { - SWSS_LOG_ENTER(); + Port port; - for (auto it = m_portList.begin(); it != m_portList.end(); ++it) + for (auto it = m_port_state_poll.begin(); it != m_port_state_poll.end(); ++it) + { + if ((it->second == 0) || !getPort(it->first, port)) + { + m_port_state_poll.erase(it); + continue; + } + if (port.m_admin_state_up && (it->second & PORT_STATE_POLL_LT)) + { + updatePortStateLinkTraining(port); + } + } + if (m_port_state_poll.size() == 0) { - updatePortLinkTrainingStatus(it->second); + m_timer->stop(); } } diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index fc2f2f81b72..161156c5be0 100755 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -185,7 +185,6 @@ class PortsOrch : public Orch, public Subject unique_ptr m_flexCounterTable; unique_ptr m_flexCounterGroupTable; Table m_portStateTable; - Table m_stateLinkTrainingTable; std::string getQueueWatermarkFlexCounterTableKey(std::string s); std::string getPriorityGroupWatermarkFlexCounterTableKey(std::string s); @@ -294,8 +293,7 @@ class PortsOrch : public Orch, public Subject bool initPort(const string &alias, const string &role, const int index, const set &lane_set); void deInitPort(string alias, sai_object_id_t port_id); - void initPortCapabilityLinkTraining(const Port &port); - bool getPortCapabilityLinkTraining(const Port& port); + void initPortCapLinkTraining(Port &port); bool setPortAdminStatus(Port &port, bool up); bool getPortAdminStatus(sai_object_id_t id, bool& up); @@ -345,7 +343,16 @@ class PortsOrch : public Orch, public Subject bool getPortLinkTrainingRxStatus(const Port &port, sai_port_link_training_rx_status_t &rx_status); bool getPortLinkTrainingFailure(const Port &port, sai_port_link_training_failure_status_t &failure); - void updatePortLinkTrainingStatus(const Port &port); + + typedef enum { + PORT_STATE_POLL_NONE = 0, + PORT_STATE_POLL_AN = 0x00000001, /* Auto Negotiation */ + PORT_STATE_POLL_LT = 0x00000002 /* Link Trainig */ + } port_state_poll_t; + + map m_port_state_poll; + void updatePortStatePoll(const Port &port, port_state_poll_t type, bool set); + void updatePortStateLinkTraining(const Port &port); void getPortSerdesVal(const std::string& s, std::vector &lane_values); bool getPortAdvSpeedsVal(const std::string &s, std::vector &speed_values); diff --git a/tests/test_port_lt.py b/tests/test_port_lt.py index 98e1251a1db..3ec51ed68b1 100644 --- a/tests/test_port_lt.py +++ b/tests/test_port_lt.py @@ -19,12 +19,12 @@ def test_PortLinkTrainingForce(self, dvs, testlog): fvs = swsscommon.FieldValuePairs([("link_training","on")]) tbl.set("Ethernet4", fvs) - # validate if autoneg false is pushed to asic db when set first time + # validate if link_training false is pushed to asic db when set first time port_oid = adb.port_name_map["Ethernet0"] expected_fields = {"SAI_PORT_ATTR_LINK_TRAINING_ENABLE":"false"} adb.wait_for_field_match("ASIC_STATE:SAI_OBJECT_TYPE_PORT", port_oid, expected_fields) - # validate if autoneg true is pushed to asic db when set first time + # validate if link_training true is pushed to asic db when set first time port_oid = adb.port_name_map["Ethernet4"] expected_fields = {"SAI_PORT_ATTR_LINK_TRAINING_ENABLE":"true"} adb.wait_for_field_match("ASIC_STATE:SAI_OBJECT_TYPE_PORT", port_oid, expected_fields)