From e9eeb9a87f27740ee692e0a1e86e4a10dd9e943f Mon Sep 17 00:00:00 2001
From: Dante Su <dante.su@broadcom.com>
Date: Thu, 28 Apr 2022 15:00:04 +0000
Subject: [PATCH] address review comments

Signed-off-by: Dante Su <dante.su@broadcom.com>
---
 orchagent/portsorch.cpp | 23 +++++++++++------------
 orchagent/portsorch.h   |  2 +-
 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp
index e900bc52e6..3a1549149f 100755
--- a/orchagent/portsorch.cpp
+++ b/orchagent/portsorch.cpp
@@ -328,7 +328,7 @@ PortsOrch::PortsOrch(DBConnector *db, DBConnector *stateDb, vector<table_name_wi
         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),
-        m_timer(new SelectableTimer(timespec { .tv_sec = PORT_STATE_POLLING_SEC, .tv_nsec = 0 }))
+        m_port_state_poller(new SelectableTimer(timespec { .tv_sec = PORT_STATE_POLLING_SEC, .tv_nsec = 0 }))
 {
     SWSS_LOG_ENTER();
 
@@ -594,7 +594,7 @@ PortsOrch::PortsOrch(DBConnector *db, DBConnector *stateDb, vector<table_name_wi
         m_lagIdAllocator = unique_ptr<LagIdAllocator> (new LagIdAllocator(chassisAppDb));
     }
 
-    auto executor = new ExecutableTimer(m_timer, this, "PORT_STATE_POLL");
+    auto executor = new ExecutableTimer(m_port_state_poller, this, "PORT_STATE_POLLER");
     Orch::addExecutor(executor);
 }
 
@@ -1945,7 +1945,7 @@ void PortsOrch::initPortCapLinkTraining(Port &port)
      */
     attr.id = SAI_PORT_ATTR_SUPPORTED_AUTO_NEG_MODE;
 #endif
-    status = sai_port_api->set_port_attribute(port.m_port_id, &attr);
+    status = sai_port_api->get_port_attribute(port.m_port_id, 1, &attr);
     if (status == SAI_STATUS_SUCCESS)
     {
         port.m_cap_lt = attr.value.booldata ? 1 : 0;
@@ -2242,12 +2242,11 @@ task_process_status PortsOrch::setPortLinkTraining(const Port &port, bool state)
     sai_status_t status = sai_port_api->set_port_attribute(port.m_port_id, &attr);
     if (status != SAI_STATUS_SUCCESS)
     {
-        SWSS_LOG_ERROR("Failed to set LT %s to port pid:%" PRIx64,
-                       op.c_str(), port.m_port_id);
+        SWSS_LOG_ERROR("Failed to set LT %s to port %s", op.c_str(), port.m_alias.c_str());
         return handleSaiSetStatus(SAI_API_PORT, status);
     }
 
-    SWSS_LOG_INFO("Set LT %s to port pid:%" PRIx64, op.c_str(), port.m_port_id);
+    SWSS_LOG_INFO("Set LT %s to port %s", op.c_str(), port.m_alias.c_str());
 
     return task_success;
 }
@@ -6153,8 +6152,6 @@ bool PortsOrch::getPortLinkTrainingFailure(const Port &port, sai_port_link_train
 
     sai_attribute_t attr;
     attr.id = SAI_PORT_ATTR_LINK_TRAINING_FAILURE_STATUS;
-    attr.value.u32 = 0;
-
     sai_status_t ret = sai_port_api->get_port_attribute(port.m_port_id, 1, &attr);
     if (ret != SAI_STATUS_SUCCESS)
     {
@@ -7202,7 +7199,7 @@ void PortsOrch::updatePortStatePoll(const Port &port, port_state_poll_t type, bo
     if (active)
     {
         m_port_state_poll[port.m_alias] |= type;
-        m_timer->start();
+        m_port_state_poller->start();
     }
     else
     {
@@ -7214,24 +7211,26 @@ void PortsOrch::doTask(swss::SelectableTimer &timer)
 {
     Port port;
 
-    for (auto it = m_port_state_poll.begin(); it != m_port_state_poll.end(); ++it)
+    for (auto it = m_port_state_poll.begin(); it != m_port_state_poll.end(); )
     {
         if ((it->second == 0) || !getPort(it->first, port))
         {
-            m_port_state_poll.erase(it);
+            it = m_port_state_poll.erase(it);
             continue;
         }
         if (!port.m_admin_state_up)
         {
+            ++it;
             continue;
         }
         if (it->second & PORT_STATE_POLL_LT)
         {
             updatePortStateLinkTraining(port);
         }
+        ++it;
     }
     if (m_port_state_poll.size() == 0)
     {
-        m_timer->stop();
+        m_port_state_poller->stop();
     }
 }
diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h
index 2114645a61..c3bd83ed38 100755
--- a/orchagent/portsorch.h
+++ b/orchagent/portsorch.h
@@ -249,7 +249,7 @@ class PortsOrch : public Orch, public Subject
 
     NotificationConsumer* m_portStatusNotificationConsumer;
 
-    swss::SelectableTimer *m_timer = nullptr;
+    swss::SelectableTimer *m_port_state_poller = nullptr;
 
     void doTask() override;
     void doTask(Consumer &consumer);