Addressing review comments:

1. use statedb PORT_TABLE and statedb LAG_TABLE to get admin and mtu on front panel and port channel interfaces. Portsyncd and teamsyncd now updates admin and MTU status of corresponding netdev to statedb PORT_TABLE and LAG_TABLE. 2. Subinterface library moved to swss/lib as part of PR sonic-net#2017. Makefile and header inclusion changes updates accordingly.
preetham-singh · Nov 12, 2021 · ed4877b · ed4877b
1 parent 0e0c8af
commit ed4877b
Show file tree

Hide file tree

Showing 11 changed files with 53 additions and 21 deletions.
diff --git a/cfgmgr/Makefile.am b/cfgmgr/Makefile.am
@@ -38,7 +38,7 @@ portmgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI)
 portmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI)
 portmgrd_LDADD = $(COMMON_LIBS) $(SAIMETA_LIBS)
 
-intfmgrd_SOURCES = intfmgrd.cpp intfmgr.cpp $(top_srcdir)/orchagent/orch.cpp $(top_srcdir)/orchagent/request_parser.cpp shellcmd.h
+intfmgrd_SOURCES = intfmgrd.cpp intfmgr.cpp $(top_srcdir)/orchagent/orch.cpp $(top_srcdir)/orchagent/request_parser.cpp $(top_srcdir)/lib/subintf.cpp shellcmd.h
 intfmgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI)
 intfmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI)
 intfmgrd_LDADD = $(COMMON_LIBS) $(SAIMETA_LIBS)

diff --git a/cfgmgr/intfmgr.cpp b/cfgmgr/intfmgr.cpp
@@ -10,7 +10,7 @@
 #include "macaddress.h"
 #include "warm_restart.h"
 #include "subscriberstatetable.h"
-#include "ifcommon.h"
+#include "subintf.h"
 
 using namespace std;
 using namespace swss;
@@ -24,7 +24,7 @@ using namespace swss;
 #define VRF_MGMT            "mgmt"
 
 #define LOOPBACK_DEFAULT_MTU_STR "65536"
-#define PORT_MTU_DEFAULT    9100
+#define DEFAULT_MTU_STR 9100
 
 IntfMgr::IntfMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, const vector<string> &tableNames) :
         Orch(cfgDb, tableNames),
@@ -40,16 +40,21 @@ IntfMgr::IntfMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, c
         m_appIntfTableProducer(appDb, APP_INTF_TABLE_NAME),
         m_appLagTable(appDb, APP_LAG_TABLE_NAME)
 {
-    auto subscriberAppTable = new swss::SubscriberStateTable(appDb,
+    /*auto subscriberAppTable = new swss::SubscriberStateTable(appDb,
             APP_LAG_TABLE_NAME, TableConsumable::DEFAULT_POP_BATCH_SIZE, 100);
     auto appConsumer = new Consumer(subscriberAppTable, this, APP_LAG_TABLE_NAME);
-    Orch::addExecutor(appConsumer);
+    Orch::addExecutor(appConsumer);*/
 
     auto subscriberStateTable = new swss::SubscriberStateTable(stateDb,
-            STATE_PORT_TABLE_NAME, TableConsumable::DEFAULT_POP_BATCH_SIZE, 200);
+            STATE_PORT_TABLE_NAME, TableConsumable::DEFAULT_POP_BATCH_SIZE, 100);
     auto stateConsumer = new Consumer(subscriberStateTable, this, STATE_PORT_TABLE_NAME);
     Orch::addExecutor(stateConsumer);
 
+    auto subscriberStateLagTable = new swss::SubscriberStateTable(stateDb,
+            STATE_LAG_TABLE_NAME, TableConsumable::DEFAULT_POP_BATCH_SIZE, 200);
+    auto stateLagConsumer = new Consumer(subscriberStateLagTable, this, STATE_LAG_TABLE_NAME);
+    Orch::addExecutor(stateLagConsumer);
+
     if (!WarmStart::isWarmStart())
     {
         flushLoopbackIntfs();
@@ -302,7 +307,7 @@ void IntfMgr::addHostSubIntf(const string&intf, const string &subIntf, const str
 }
 
 
-std::string IntfMgr::getPortAdminStatus(const string &alias)
+std::string IntfMgr::getIntfAdminStatus(const string &alias)
 {
     Table *portTable;
     if (!alias.compare(0, strlen("Eth"), "Eth"))
@@ -362,7 +367,7 @@ std::string IntfMgr::getPortMtu(const string &alias)
         }
     }
     if (mtu.empty())
-        mtu = std::to_string(PORT_MTU_DEFAULT);
+        mtu = std::to_string(DEFAULT_MTU_STR);
     return mtu;
 }
 
@@ -376,7 +381,13 @@ void IntfMgr::updateSubIntfMtu(const string &alias, const string &mtu)
         if (subIf.parentIntf() == alias)
         {
             std::vector<FieldValueTuple> fvVector;
-            string subintf_mtu = setHostSubIntfMtu(intf, m_subIntfList[intf].mtu, mtu);
+
+            string subif_config_mtu = m_subIntfList[intf].mtu;
+            if (subif_config_mtu == MTU_INHERITANCE || subif_config_mtu.empty())
+                subif_config_mtu = std::to_string(DEFAULT_MTU_STR);
+
+            string subintf_mtu = setHostSubIntfMtu(intf, subif_config_mtu, mtu);
+
             FieldValueTuple fvTuple("mtu", subintf_mtu);
             fvVector.push_back(fvTuple);
             m_appIntfTableProducer.set(intf, fvVector);
@@ -399,6 +410,7 @@ std::string IntfMgr::setHostSubIntfMtu(const string &alias, const string &config
     {
         subifMtu = parentMtu;
     }
+    SWSS_LOG_INFO("subintf %s active mtu: %s", alias.c_str(), subifMtu.c_str());
     cmd << IP_CMD " link set " << shellquote(alias) << " mtu " << shellquote(subifMtu);
     EXEC_WITH_ERROR_THROW(cmd.str(), res);
 
@@ -436,6 +448,7 @@ std::string IntfMgr::setHostSubIntfAdminStatus(const string &alias, const string
 
     if (parentAdmin == "up" || configAdmin == "down")
     {
+        SWSS_LOG_INFO("subintf %s admin_status: %s", alias.c_str(), configAdmin.c_str());
         cmd << IP_CMD " link set " << shellquote(alias) << " " << shellquote(configAdmin);
         EXEC_WITH_ERROR_THROW(cmd.str(), res);
         return configAdmin;
@@ -790,7 +803,7 @@ bool IntfMgr::doIntfGeneralTask(const vector<string>& keys,
             }
             try
             {
-                string parentAdmin = getPortAdminStatus(subIf.parentIntf());
+                string parentAdmin = getIntfAdminStatus(subIf.parentIntf());
                 string subintf_admin = setHostSubIntfAdminStatus(alias, adminStatus, parentAdmin);
                 m_subIntfList[alias].currAdminStatus = subintf_admin;
                 FieldValueTuple fvTuple("admin_status", adminStatus);
@@ -960,7 +973,7 @@ void IntfMgr::doTask(Consumer &consumer)
     while (it != consumer.m_toSync.end())
     {
         KeyOpFieldsValuesTuple t = it->second;
-        if ((table_name == STATE_PORT_TABLE_NAME) || (table_name == APP_LAG_TABLE_NAME))
+        if ((table_name == STATE_PORT_TABLE_NAME) || (table_name == STATE_LAG_TABLE_NAME))
         {
             doPortTableTask(kfvKey(t), kfvFieldsValues(t), kfvOp(t));
         }

diff --git a/cfgmgr/intfmgr.h b/cfgmgr/intfmgr.h
@@ -57,7 +57,7 @@ class IntfMgr : public Orch
     void delLoopbackIntf(const std::string &alias);
     void flushLoopbackIntfs(void);
 
-    std::string getPortAdminStatus(const std::string &alias);
+    std::string getIntfAdminStatus(const std::string &alias);
     std::string getPortMtu(const std::string &alias);
     void addHostSubIntf(const std::string&intf, const std::string &subIntf, const std::string &vlan);
     std::string setHostSubIntfMtu(const std::string &alias, const std::string &configMtu, const std::string &parentMtu);

diff --git a/cfgmgr/portmgr.cpp b/cfgmgr/portmgr.cpp
@@ -35,7 +35,6 @@ bool PortMgr::setPortMtu(const string &alias, const string &mtu)
     fvs.push_back(fv);
     m_appPortTable.set(alias, fvs);
 
-    m_statePortTable.set(alias, fvs);
     return true;
 }
 
@@ -68,7 +67,6 @@ bool PortMgr::setPortAdminStatus(const string &alias, const bool up)
     fvs.push_back(fv);
     m_appPortTable.set(alias, fvs);
 
-    m_statePortTable.set(alias, fvs);
     return true;
 }
 

diff --git a/orchagent/Makefile.am b/orchagent/Makefile.am
@@ -34,6 +34,7 @@ endif
 orchagent_SOURCES = \
             main.cpp \
             $(top_srcdir)/lib/gearboxutils.cpp \
+            $(top_srcdir)/lib/subintf.cpp \
             orchdaemon.cpp \
             orch.cpp \
             notifications.cpp \

diff --git a/orchagent/intfsorch.cpp b/orchagent/intfsorch.cpp
@@ -662,6 +662,7 @@ void IntfsOrch::doTask(Consumer &consumer)
         string proxy_arp = "";
         string inband_type = "";
         bool mpls = false;
+        string vlan = "";
 
         for (auto idx : data)
         {

diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp
@@ -5,6 +5,7 @@
 #include "gearboxutils.h"
 #include "vxlanorch.h"
 #include "directory.h"
+#include "subintf.h"
 
 #include <inttypes.h>
 #include <cassert>
@@ -31,7 +32,6 @@
 #include "fdborch.h"
 #include "stringutility.h"
 #include "subscriberstatetable.h"
-#include "ifcommon.h"
 
 extern sai_switch_api_t *sai_switch_api;
 extern sai_bridge_api_t *sai_bridge_api;

diff --git a/portsyncd/linksync.cpp b/portsyncd/linksync.cpp
@@ -182,6 +182,7 @@ void LinkSync::onMsg(int nlmsg_type, struct nl_object *obj)
     unsigned int ifindex = rtnl_link_get_ifindex(link);
     int master = rtnl_link_get_master(link);
     char *type = rtnl_link_get_type(link);
+    unsigned int mtu = rtnl_link_get_mtu(link);
 
     if (type)
     {
@@ -251,10 +252,14 @@ void LinkSync::onMsg(int nlmsg_type, struct nl_object *obj)
     {
         g_portSet.erase(key);
         FieldValueTuple tuple("state", "ok");
+        FieldValueTuple admin_status("admin_status", (admin ? "up" : "down"));
+        FieldValueTuple port_mtu("mtu", to_string(mtu));
         vector<FieldValueTuple> vector;
         vector.push_back(tuple);
         FieldValueTuple op("netdev_oper_status", oper ? "up" : "down");
         vector.push_back(op);
+        vector.push_back(admin_status);
+        vector.push_back(port_mtu);
         m_statePortTable.set(key, vector);
         SWSS_LOG_NOTICE("Publish %s(ok:%s) to state db", key.c_str(), oper ? "up" : "down");
     }

diff --git a/teamsyncd/teamsync.cpp b/teamsyncd/teamsync.cpp
@@ -157,11 +157,19 @@ void TeamSync::addLag(const string &lagName, int ifindex, bool admin_state,
     SWSS_LOG_INFO("Add %s admin_status:%s oper_status:%s, mtu: %d",
                    lagName.c_str(), admin_state ? "up" : "down", oper_state ? "up" : "down", mtu);
 
+    bool lag_update = true;
+    auto it = m_teamSelectables.find(lagName);
     /* Return when the team instance has already been tracked */
     if (m_teamSelectables.find(lagName) != m_teamSelectables.end())
-        return;
+    {
+        auto tsync = m_teamSelectables[lagName];
+        if (tsync->admin_state == admin_state && tsync->mtu == mtu)
+            return;
+        tsync->admin_state = admin_state;
+        tsync->mtu = mtu;
+        lag_update = false;
+    }
 
-    fvVector.clear();
     FieldValueTuple s("state", "ok");
     fvVector.push_back(s);
     if (m_warmstart)
@@ -173,10 +181,13 @@ void TeamSync::addLag(const string &lagName, int ifindex, bool admin_state,
         m_stateLagTable.set(lagName, fvVector);
     }
 
-    /* Create the team instance */
-    auto sync = make_shared<TeamPortSync>(lagName, ifindex, &m_lagMemberTable);
-    m_teamSelectables[lagName] = sync;
-    m_selectablesToAdd.insert(lagName);
+    if (lag_update)
+    {
+        /* Create the team instance */
+        auto sync = make_shared<TeamPortSync>(lagName, ifindex, &m_lagMemberTable);
+        m_teamSelectables[lagName] = sync;
+        m_selectablesToAdd.insert(lagName);
+    }
 }
 
 void TeamSync::removeLag(const string &lagName)

diff --git a/teamsyncd/teamsync.h b/teamsyncd/teamsync.h
@@ -43,6 +43,8 @@ class TeamSync : public NetMsg
 
         /* member_name -> enabled|disabled */
         std::map<std::string, bool> m_lagMembers;
+        bool admin_state;
+        unsigned int mtu;
     protected:
         int onChange();
         static int teamdHandler(struct team_handle *th, void *arg,

diff --git a/tests/mock_tests/Makefile.am b/tests/mock_tests/Makefile.am
@@ -33,6 +33,7 @@ tests_SOURCES = aclorch_ut.cpp \
                 mock_redisreply.cpp \
                 bulker_ut.cpp \
                 $(top_srcdir)/lib/gearboxutils.cpp \
+                $(top_srcdir)/lib/subintf.cpp \
                 $(top_srcdir)/orchagent/orchdaemon.cpp \
                 $(top_srcdir)/orchagent/orch.cpp \
                 $(top_srcdir)/orchagent/notifications.cpp \