[portmgrd]: Fix setting default port admin status and MTU (sonic-net#691

) This commit moves setting of default admin status and mtu to portmgrd to be aligned with other managers (e.g. teammgrd). It also fixes "N/A" admin status in "show interface status" output for ports which have no 'admin_status' field in config DB. Signed-off-by: Stepan Blyschak <[email protected]>
lguohan · Nov 21, 2018 · 6007e7f · 6007e7f
1 parent 6c70f6d
commit 6007e7f
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 15 deletions.
diff --git a/cfgmgr/portmgr.cpp b/cfgmgr/portmgr.cpp
@@ -10,6 +10,10 @@
 using namespace std;
 using namespace swss;
 
+/* Port default admin status is down */
+#define DEFAULT_ADMIN_STATUS_STR    "down"
+#define DEFAULT_MTU_STR             "9100"
+
 PortMgr::PortMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, const vector<string> &tableNames) :
         Orch(cfgDb, tableNames),
         m_cfgPortTable(cfgDb, CFG_PORT_TABLE_NAME),
@@ -91,23 +95,44 @@ void PortMgr::doTask(Consumer &consumer)
                 continue;
             }
 
+            string admin_status, mtu;
+
+            bool configured = (m_portList.find(alias) != m_portList.end());
+
+            /* If this is the first time we set port settings
+             * assign default admin status and mtu
+             */
+            if (!configured)
+            {
+                admin_status = DEFAULT_ADMIN_STATUS_STR;
+                mtu = DEFAULT_MTU_STR;
+
+                m_portList.insert(alias);
+            }
+
             for (auto i : kfvFieldsValues(t))
             {
                 if (fvField(i) == "mtu")
                 {
-                    auto mtu = fvValue(i);
-                    setPortMtu(alias, mtu);
-                    SWSS_LOG_NOTICE("Configure %s MTU to %s",
-                                    alias.c_str(), mtu.c_str());
+                    mtu = fvValue(i);
                 }
                 else if (fvField(i) == "admin_status")
                 {
-                    auto status = fvValue(i);
-                    setPortAdminStatus(alias, status == "up");
-                    SWSS_LOG_NOTICE("Configure %s %s",
-                            alias.c_str(), status.c_str());
+                    admin_status = fvValue(i);
                 }
             }
+
+            if (!mtu.empty())
+            {
+                setPortMtu(alias, mtu);
+                SWSS_LOG_NOTICE("Configure %s MTU to %s", alias.c_str(), mtu.c_str());
+            }
+
+            if (!admin_status.empty())
+            {
+                setPortAdminStatus(alias, admin_status == "up");
+                SWSS_LOG_NOTICE("Configure %s admin status to %s", alias.c_str(), admin_status.c_str());
+            }
         }
 
         it = consumer.m_toSync.erase(it);

diff --git a/cfgmgr/portmgr.h b/cfgmgr/portmgr.h
@@ -22,6 +22,8 @@ class PortMgr : public Orch
     Table m_statePortTable;
     ProducerStateTable m_appPortTable;
 
+    set<string> m_portList;
+
     void doTask(Consumer &consumer);
     bool setPortMtu(const string &alias, const string &mtu);
     bool setPortAdminStatus(const string &alias, const bool up);

diff --git a/cfgmgr/teammgr.h b/cfgmgr/teammgr.h
@@ -28,7 +28,6 @@ class TeamMgr : public Orch
     ProducerStateTable m_appPortTable;
     ProducerStateTable m_appLagTable;
 
-    set<string> m_portList;
     set<string> m_lagList;
 
     MacAddress m_mac;

diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp
@@ -2286,12 +2286,6 @@ bool PortsOrch::initializePort(Port &p)
     }
     SWSS_LOG_DEBUG("initializePort %s with admin %s and oper %s", p.m_alias.c_str(), adminStatus.c_str(), operStatus.c_str());
 
-    /* Set port admin status to DOWN if attr missing */
-    if (adminStatus != "up")
-    {
-        setPortAdminStatus(p.m_port_id, false);
-    }
-
     /**
      * Create database port oper status as DOWN if attr missing
      * This status will be updated when receiving port_oper_status_notification.