orchagent: Adding port_status_change handling function (sonic-net#86)

* orchagent: Adding port_status_change notification handling function
- Write back to APPL_DB to indicate port oper status
- Fixing log levels in various places

* orchagent: Adding setHostIntfsOperStatus function
- Add unique_ptr

orchagent/Makefile.am

@@ -10,7 +10,7 @@ else
 DBGFLAGS = -g -DNDEBUG
 endif
 
-orchagent_SOURCES = main.cpp orchdaemon.cpp orch.cpp routeorch.cpp neighorch.cpp intfsorch.cpp portsorch.cpp copporch.cpp tunneldecaporch.cpp qosorch.cpp bufferorch.cpp
+orchagent_SOURCES = main.cpp orchdaemon.cpp orch.cpp notifications.cpp routeorch.cpp neighorch.cpp intfsorch.cpp portsorch.cpp copporch.cpp tunneldecaporch.cpp qosorch.cpp bufferorch.cpp
 
 orchagent_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI)
 orchagent_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI)

orchagent/intfsorch.cpp

@@ -67,7 +67,7 @@ void IntfsOrch::doTask(Consumer &consumer)
             Port port;
             if (!gPortsOrch->getPort(alias, port))
             {
-                SWSS_LOG_INFO("Failed to locate interface %s\n", alias.c_str());
+                /* TODO: Resolve the dependency relationship and add ref_count to port */
                 it++;
                 continue;
             }

orchagent/main.cpp

@@ -17,6 +17,8 @@ extern "C" {
 using namespace std;
 using namespace swss;
 
+extern sai_switch_notification_t switch_notifications;
+
 #define UNREFERENCED_PARAMETER(P)       (P)
 
 /* Initialize all global api pointers */
@@ -74,9 +76,6 @@ const service_method_table_t test_services = {
     test_profile_get_next_value
 };
 
-sai_switch_notification_t switch_notifications = {
-};
-
 void initSaiApi()
 {
     SWSS_LOG_ENTER();

orchagent/notifications.cpp

@@ -0,0 +1,35 @@
+#include "portsorch.h"
+
+extern "C" {
+#include "sai.h"
+}
+
+#include "logger.h"
+
+extern PortsOrch *gPortsOrch;
+
+void on_port_state_change(uint32_t count, sai_port_oper_status_notification_t *data)
+{
+    /* Wait until gPortsOrch is initialized */
+    if (!gPortsOrch || !gPortsOrch->isInitDone())
+        return;
+
+    for (uint32_t i = 0; i < count; i++)
+    {
+        sai_object_id_t id = data[i].port_id;
+        sai_port_oper_status_t status = data[i].port_state;
+
+        gPortsOrch->updateDbPortOperStatus(id, status);
+        gPortsOrch->setHostIntfsOperStatus(id, status == SAI_PORT_OPER_STATUS_UP);
+    }
+}
+
+sai_switch_notification_t switch_notifications
+{
+    NULL,
+    NULL,
+    on_port_state_change,
+    NULL,
+    NULL,
+    NULL
+};

orchagent/orch.h

@@ -2,6 +2,7 @@
 #define SWSS_ORCH_H
 
 #include <map>
+#include <memory>
 
 extern "C" {
 #include "sai.h"
@@ -69,9 +70,9 @@ class Orch
     bool execute(string tableName);
     /* Iterate all consumers in m_consumerMap and run doTask(Consumer) */
     void doTask();
-private:
-    DBConnector *m_db;
+
 protected:
+    DBConnector *m_db;
     ConsumerMap m_consumerMap;
 
     /* Run doTask against a specific consumer */

orchagent/port.h

@@ -3,7 +3,6 @@
 
 extern "C" {
 #include "sai.h"
-#include "saistatus.h"
 }
 
 #include <set>

orchagent/portsorch.cpp

@@ -24,9 +24,12 @@ PortsOrch::PortsOrch(DBConnector *db, vector<string> tableNames) :
 {
     SWSS_LOG_ENTER();
 
-    /* Initialize Counter Table */
-    DBConnector *counter_db = new DBConnector(COUNTERS_DB, "localhost", 6379, 0);
-    m_counterTable = new Table(counter_db, COUNTERS_PORT_NAME_MAP);
+    /* Initialize counter table */
+    unique_ptr<DBConnector> pCounter_db(new DBConnector(COUNTERS_DB, "localhost", 6379, 0));
+    m_counterTable = unique_ptr<Table>(new Table(pCounter_db.get(), COUNTERS_PORT_NAME_MAP));
+
+    /* Initialize port table */
+    m_portTable = unique_ptr<Table>(new Table(m_db, APP_PORT_TABLE_NAME));
 
     int i, j;
     sai_status_t status;
@@ -175,12 +178,56 @@ bool PortsOrch::setPortAdminStatus(sai_object_id_t id, bool up)
     sai_status_t status = sai_port_api->set_port_attribute(id, &attr);
     if (status != SAI_STATUS_SUCCESS)
     {
+        SWSS_LOG_ERROR("Failed to set admin status %s to port pid:%llx",
+                       up ? "UP" : "DOWN", id);
         return false;
     }
-
     return true;
 }
 
+bool PortsOrch::setHostIntfsOperStatus(sai_object_id_t port_id, bool up)
+{
+    SWSS_LOG_ENTER();
+
+    for (auto it = m_portList.begin(); it != m_portList.end(); it++)
+    {
+        if (it->second.m_port_id == port_id)
+        {
+            sai_attribute_t attr;
+            attr.id = SAI_HOSTIF_ATTR_OPER_STATUS;
+            attr.value.booldata = up;
+
+            sai_status_t status = sai_hostif_api->set_hostif_attribute(it->second.m_hif_id, &attr);
+            if (status != SAI_STATUS_SUCCESS)
+            {
+                SWSS_LOG_WARN("Failed to set operation status %s to host interface %s",
+                              up ? "UP" : "DOWN", it->second.m_alias.c_str());
+                return false;
+            }
+            SWSS_LOG_NOTICE("Set operation status %s to host interface %s",
+                            up ? "UP" : "DOWN", it->second.m_alias.c_str());
+            return true;
+        }
+    }
+    return false;
+}
+
+void PortsOrch::updateDbPortOperStatus(sai_object_id_t id, sai_port_oper_status_t status)
+{
+    SWSS_LOG_ENTER();
+
+    for (auto it = m_portList.begin(); it != m_portList.end(); it++)
+    {
+        if (it->second.m_port_id == id)
+        {
+            vector<FieldValueTuple> vector;
+            FieldValueTuple tuple("oper_status", to_string(status));
+            vector.push_back(tuple);
+            m_portTable->set(it->first, vector);
+        }
+    }
+}
+
 void PortsOrch::doPortTask(Consumer &consumer)
 {
     SWSS_LOG_ENTER();
@@ -607,11 +654,7 @@ bool PortsOrch::initializePort(Port &p)
     initializeQueues(p);
 
     /* Set up host interface */
-    if (!addHostIntfs(p.m_port_id, p.m_alias, p.m_hif_id))
-    {
-        SWSS_LOG_ERROR("Failed to set up host interface pid:%llx alias:%s", p.m_port_id, p.m_alias.c_str());
-        return false;
-    }
+    addHostIntfs(p.m_port_id, p.m_alias, p.m_hif_id);
 
     // TODO: Assure if_nametoindex(p.m_alias.c_str()) != 0
     // TODO: Get port oper status
@@ -626,11 +669,8 @@ bool PortsOrch::initializePort(Port &p)
 #endif
 
     /* Set port admin status UP */
-    if (!setPortAdminStatus(p.m_port_id, true))
-    {
-        SWSS_LOG_ERROR("Failed to set port admin status UP pid:%llx", p.m_port_id);
-        return false;
-    }
+    setPortAdminStatus(p.m_port_id, true);
+
     return true;
 }
 
@@ -656,7 +696,7 @@ bool PortsOrch::addHostIntfs(sai_object_id_t id, string alias, sai_object_id_t &
     sai_status_t status = sai_hostif_api->create_hostif(&host_intfs_id, attrs.size(), attrs.data());
     if (status != SAI_STATUS_SUCCESS)
     {
-        SWSS_LOG_ERROR("Failed to create host interface");
+        SWSS_LOG_ERROR("Failed to create host interface for port %s", alias.c_str());
         return false;
     }
 

orchagent/portsorch.h

@@ -19,8 +19,11 @@ class PortsOrch : public Orch
     void setPort(string alias, Port port);
     sai_object_id_t getCpuPort();
 
+    bool setHostIntfsOperStatus(sai_object_id_t id, bool up);
+    void updateDbPortOperStatus(sai_object_id_t id, sai_port_oper_status_t status);
 private:
-    Table *m_counterTable;
+    unique_ptr<Table> m_counterTable;
+    unique_ptr<Table> m_portTable;
 
     bool m_initDone = false;
     sai_object_id_t m_cpuPort;

portsyncd/linksync.cpp

@@ -61,8 +61,8 @@ void LinkSync::onMsg(int nlmsg_type, struct nl_object *obj)
         return;
 
     unsigned int flags = rtnl_link_get_flags(link);
-    bool admin_state = flags & IFF_UP;
-    bool oper_state = flags & IFF_LOWER_UP;
+    bool admin = flags & IFF_UP;
+    bool oper = flags & IFF_LOWER_UP;
     unsigned int mtu = rtnl_link_get_mtu(link);
 
     char addrStr[MAX_ADDR_SIZE+1] = {0};
@@ -72,12 +72,12 @@ void LinkSync::onMsg(int nlmsg_type, struct nl_object *obj)
     int master = rtnl_link_get_master(link);
     char *type = rtnl_link_get_type(link);
 
-    cout << "Receive nlmsg from portsyncd: type:" << nlmsg_type << " key:" << key
-         << " admin_state:" << admin_state << " oper_state:" << oper_state
-         << " addr:" << addrStr << " ifindex:" << ifindex << " master:" << master;
     if (type)
-        cout << " type:" << type;
-    cout << endl;
+        SWSS_LOG_DEBUG("nlmsg type:%d key:%s admin:%d oper:%d addr:%s ifindex:%d master:%d type:%s",
+                       nlmsg_type, key.c_str(), admin, oper, addrStr, ifindex, master, type);
+    else
+        SWSS_LOG_DEBUG("nlmsg type:%d key:%s admin:%d oper:%d addr:%s ifindex:%d master:%d",
+                       nlmsg_type, key.c_str(), admin, oper, addrStr, ifindex, master);
 
     /* Insert or update the ifindex to key map */
     m_ifindexNameMap[ifindex] = key;
@@ -104,8 +104,8 @@ void LinkSync::onMsg(int nlmsg_type, struct nl_object *obj)
         }
     }
 
-    FieldValueTuple a("admin_status", admin_state ? "up" : "down");
-    FieldValueTuple o("oper_status", oper_state ? "up" : "down");
+    FieldValueTuple a("admin_status", admin ? "up" : "down");
+    FieldValueTuple o("oper_status", oper ? "up" : "down");
     FieldValueTuple m("mtu", to_string(mtu));
     fvVector.push_back(a);
     fvVector.push_back(o);
@@ -141,6 +141,4 @@ void LinkSync::onMsg(int nlmsg_type, struct nl_object *obj)
 
         return;
     }
-
-    cerr << "Unhandled netlink message received." << endl;
 }