[Vlans]: Extract VLAN_MEMBER_TABLE from VLAN_TABLE

doc/swss-schema.md

@@ -101,14 +101,18 @@ For example (reorder output)
 ### VLAN_TABLE
     ;Defines VLANs and the interfaces which are members of the vlan
     ;Status: work in progress
-    key                 = VLAN_TABLE:"vlan"vlanid ; DIGIT 0-4095 with prefix "Vlan"
+    key                 = VLAN_TABLE:"Vlan"vlanid ; DIGIT 0-4095 with prefix "Vlan"
     admin_status        = "down" / "up"        ; admin status
     oper_status         = "down" / "up"        ; operating status
     mtu                 = 1*4DIGIT             ; MTU for the IP interface of the VLAN
 
-    key                 = VLAN_TABLE:vlanid:ifname ; physical port member of VLAN
-    tagging_mode        = "untagged" / "tagged" / "priority_tagged" ; default value as untagged
+---------------------------------------------
+### VLAN_MEMBER_TABLE
+    ;Defines interfaces which are members of a vlan
+    ;Status: work in progress
 
+    key                 = VLAN_MEMBER_TABLE:"Vlan"vlanid:ifname ; physical port "ifname" is a member of a VLAN "VlanX"
+    tagging_mode        = "untagged" / "tagged" / "priority_tagged" ; default value as untagged
 
 ---------------------------------------------
 ### LAG_TABLE

orchagent/orchdaemon.cpp

@@ -36,6 +36,7 @@ bool OrchDaemon::init()
     vector<string> ports_tables = {
         APP_PORT_TABLE_NAME,
         APP_VLAN_TABLE_NAME,
+        APP_VLAN_MEMBER_TABLE_NAME,
         APP_LAG_TABLE_NAME,
         APP_LAG_MEMBER_TABLE_NAME
     };

orchagent/portsorch.cpp

@@ -451,117 +451,145 @@ void PortsOrch::doVlanTask(Consumer &consumer)
         /* Ensure the key starts with "Vlan" otherwise ignore */
         if (strncmp(key.c_str(), VLAN_PREFIX, 4))
         {
+            SWSS_LOG_ERROR("Invalid key format. No 'Vlan' prefix: %s", key.c_str());
             it = consumer.m_toSync.erase(it);
             continue;
         }
 
-        key = key.substr(4);
-        size_t found = key.find(':');
         int vlan_id;
-        string vlan_alias, port_alias;
-        if (found == string::npos)
-            vlan_id = stoi(key);
-        else
-        {
-            vlan_id = stoi(key.substr(0, found));
-            port_alias = key.substr(found+1);
-        }
+        vlan_id = stoi(key.substr(4)); // FIXME: might raise exception
 
+        string vlan_alias, port_alias;
         vlan_alias = VLAN_PREFIX + to_string(vlan_id);
         string op = kfvOp(t);
 
-        /* Manipulate VLAN when port_alias is empty */
-        if (port_alias == "")
+        if (op == SET_COMMAND)
         {
-            if (op == SET_COMMAND)
+            /* Duplicate entry */
+            if (m_portList.find(vlan_alias) != m_portList.end())
             {
-                /* Duplicate entry */
-                if (m_portList.find(vlan_alias) != m_portList.end())
-                {
-                    it = consumer.m_toSync.erase(it);
-                    continue;
-                }
-
-                if (addVlan(vlan_alias))
-                    it = consumer.m_toSync.erase(it);
-                else
-                    it++;
+                it = consumer.m_toSync.erase(it);
+                continue;
             }
-            else if (op == DEL_COMMAND)
-            {
-                Port vlan;
-                getPort(vlan_alias, vlan);
 
-                if (removeVlan(vlan))
-                    it = consumer.m_toSync.erase(it);
-                else
-                    it++;
-            }
+            if (addVlan(vlan_alias))
+                it = consumer.m_toSync.erase(it);
             else
-            {
-                SWSS_LOG_ERROR("Unknown operation type %s", op.c_str());
+                it++;
+        }
+        else if (op == DEL_COMMAND)
+        {
+            Port vlan;
+            getPort(vlan_alias, vlan);
+
+            if (removeVlan(vlan))
                 it = consumer.m_toSync.erase(it);
-            }
+            else
+                it++;
         }
-        /* Manipulate member */
         else
         {
-            assert(m_portList.find(vlan_alias) != m_portList.end());
-            Port vlan, port;
+            SWSS_LOG_ERROR("Unknown operation type %s", op.c_str());
+            it = consumer.m_toSync.erase(it);
+        }
+    }
+}
 
-            /* When VLAN member is to be created before VLAN is created */
-            if (!getPort(vlan_alias, vlan))
-            {
-                SWSS_LOG_INFO("Failed to locate VLAN %s", vlan_alias.c_str());
-                it++;
-                continue;
-            }
+void PortsOrch::doVlanMemberTask(Consumer &consumer)
+{
+    if (!isInitDone())
+        return;
+
+    auto it = consumer.m_toSync.begin();
+    while (it != consumer.m_toSync.end())
+    {
+        auto &t = it->second;
+
+        string key = kfvKey(t);
+
+        /* Ensure the key starts with "Vlan" otherwise ignore */
+        if (strncmp(key.c_str(), VLAN_PREFIX, 4))
+        {
+            SWSS_LOG_ERROR("Invalid key format. No 'Vlan' prefix: %s", key.c_str());
+            it = consumer.m_toSync.erase(it);
+            continue;
+        }
+
+        key = key.substr(4);
+        size_t found = key.find(':');
+        int vlan_id;
+        string vlan_alias, port_alias;
+        if (found != string::npos)
+        {
+            vlan_id = stoi(key.substr(0, found)); // FIXME: might raise exception
+            port_alias = key.substr(found+1);
+        }
+        else
+        {
+            SWSS_LOG_ERROR("Invalid key format. No member port is presented: %s",
+                           kfvKey(t).c_str());
+            it = consumer.m_toSync.erase(it);
+            continue;
+        }
 
-            if (!getPort(port_alias, port))
+        vlan_alias = VLAN_PREFIX + to_string(vlan_id);
+        string op = kfvOp(t);
+
+        assert(m_portList.find(vlan_alias) != m_portList.end());
+        Port vlan, port;
+
+        /* When VLAN member is to be created before VLAN is created */
+        if (!getPort(vlan_alias, vlan))
+        {
+            SWSS_LOG_INFO("Failed to locate VLAN %s", vlan_alias.c_str());
+            it++;
+            continue;
+        }
+
+        if (!getPort(port_alias, port))
+        {
+            SWSS_LOG_ERROR("Failed to locate port %s", port_alias.c_str());
+            it = consumer.m_toSync.erase(it);
+            continue;
+        }
+
+        if (op == SET_COMMAND)
+        {
+            /* Duplicate entry */
+            if (vlan.m_members.find(port_alias) != vlan.m_members.end())
             {
-                SWSS_LOG_ERROR("Failed to locate port %s", port_alias.c_str());
                 it = consumer.m_toSync.erase(it);
                 continue;
             }
 
-            if (op == SET_COMMAND)
-            {
-                /* Duplicate entry */
-                if (vlan.m_members.find(port_alias) != vlan.m_members.end())
-                {
-                    it = consumer.m_toSync.erase(it);
-                    continue;
-                }
+            /* Assert the port doesn't belong to any VLAN */
+            assert(!port.m_vlan_id && !port.m_vlan_member_id);
 
-                /* Assert the port doesn't belong to any VLAN */
-                assert(!port.m_vlan_id && !port.m_vlan_member_id);
+            if (addVlanMember(vlan, port))
+                it = consumer.m_toSync.erase(it);
+            else
+                it++;
+        }
+        else if (op == DEL_COMMAND)
+        {
+            if (vlan.m_members.find(port_alias) != vlan.m_members.end())
+            {
+                /* Assert the port belongs the a VLAN */
+                assert(port.m_vlan_id && port.m_vlan_member_id);
 
-                if (addVlanMember(vlan, port))
+                if (removeVlanMember(vlan, port))
                     it = consumer.m_toSync.erase(it);
                 else
                     it++;
             }
-            else if (op == DEL_COMMAND)
-            {
-                if (vlan.m_members.find(port_alias) != vlan.m_members.end())
-                {
-                    /* Assert the port belongs the a VLAN */
-                    assert(port.m_vlan_id && port.m_vlan_member_id);
-
-                    if (removeVlanMember(vlan, port))
-                        it = consumer.m_toSync.erase(it);
-                    else
-                        it++;
-                }
-                else
-                    /* Cannot locate the VLAN */
-                    it = consumer.m_toSync.erase(it);
-            }
             else
-            {
-                SWSS_LOG_ERROR("Unknown operation type %s", op.c_str());
+                /* Cannot locate the VLAN */
                 it = consumer.m_toSync.erase(it);
-            }
+        }
+        else
+        {
+            SWSS_LOG_ERROR("Unknown operation type %s", op.c_str());
+            it = consumer.m_toSync.erase(it);
         }
     }
 }
@@ -732,6 +760,8 @@ void PortsOrch::doTask(Consumer &consumer)
         doPortTask(consumer);
     else if (table_name == APP_VLAN_TABLE_NAME)
         doVlanTask(consumer);
+    else if (table_name == APP_VLAN_MEMBER_TABLE_NAME)
+        doVlanMemberTask(consumer);
     else if (table_name == APP_LAG_TABLE_NAME)
         doLagTask(consumer);
     else if (table_name == APP_LAG_MEMBER_TABLE_NAME)

orchagent/portsorch.h

@@ -63,6 +63,7 @@ class PortsOrch : public Orch, public Subject
     void doTask(Consumer &consumer);
     void doPortTask(Consumer &consumer);
     void doVlanTask(Consumer &consumer);
+    void doVlanMemberTask(Consumer &consumer);
     void doLagTask(Consumer &consumer);
     void doLagMemberTask(Consumer &consumer);
 

portsyncd/linksync.cpp

@@ -33,9 +33,10 @@ extern bool g_init;
 LinkSync::LinkSync(DBConnector *db) :
     m_portTableProducer(db, APP_PORT_TABLE_NAME),
     m_vlanTableProducer(db, APP_VLAN_TABLE_NAME),
+    m_vlanMemberTableProducer(db, APP_VLAN_MEMBER_TABLE_NAME),
     m_lagTableProducer(db, APP_LAG_TABLE_NAME),
     m_portTableConsumer(db, APP_PORT_TABLE_NAME),
-    m_vlanTableConsumer(db, APP_VLAN_TABLE_NAME),
+    m_vlanMemberTableConsumer(db, APP_VLAN_MEMBER_TABLE_NAME),
     m_lagTableConsumer(db, APP_LAG_TABLE_NAME)
 {
     /* See the comments for g_portSet in portsyncd.cpp */
@@ -56,16 +57,11 @@ LinkSync::LinkSync(DBConnector *db) :
     }
 
     vector<KeyOpFieldsValuesTuple> tuples;
-    m_vlanTableConsumer.getTableContent(tuples);
+    m_vlanMemberTableConsumer.getTableContent(tuples);
 
     for (auto tuple : tuples)
     {
         vector<string> keys = tokenize(kfvKey(tuple), ':');
-        if (keys.size() != 2)
-        {
-            continue;
-        }
-
         string vlan = keys[0];
         string member = keys[1];
 
@@ -137,15 +133,15 @@ void LinkSync::onMsg(int nlmsg_type, struct nl_object *obj)
 
             if (nlmsg_type == RTM_DELLINK) /* Will it happen? */
             {
-                m_vlanTableProducer.del(member_key);
+                m_vlanMemberTableProducer.del(member_key);
             }
             else /* RTM_NEWLINK */
             {
                 vector<FieldValueTuple> fvVector;
                 FieldValueTuple t("tagging_mode", "untagged");
                 fvVector.push_back(t);
 
-                m_vlanTableProducer.set(member_key, fvVector);
+                m_vlanMemberTableProducer.set(member_key, fvVector);
             }
         }
     }
@@ -158,7 +154,7 @@ void LinkSync::onMsg(int nlmsg_type, struct nl_object *obj)
             if (member_set.find(key) != member_set.end())
             {
                 string member_key = (*i).first + ":" + key;
-                m_vlanTableProducer.del(member_key);
+                m_vlanMemberTableProducer.del(member_key);
             }
         }
     }

portsyncd/linksync.h

@@ -19,8 +19,8 @@ class LinkSync : public NetMsg
     virtual void onMsg(int nlmsg_type, struct nl_object *obj);
 
 private:
-    ProducerStateTable m_portTableProducer, m_vlanTableProducer, m_lagTableProducer;
-    Table m_portTableConsumer, m_vlanTableConsumer, m_lagTableConsumer;
+    ProducerStateTable m_portTableProducer, m_vlanTableProducer, m_vlanMemberTableProducer, m_lagTableProducer;
+    Table m_portTableConsumer, m_vlanMemberTableConsumer, m_lagTableConsumer;
 
     std::map<unsigned int, std::string> m_ifindexNameMap;
 };