This commit contains:

1. Review comments incorporated
2. Dependency for PR #885 is resolved
3. Added test_evpn_fdb.py pytest script to verify EVPN-VXLAN-FDB
4. compiled and verified the FDB functionality with EVPN
5. Changes to handle the static mac and aging from CLI

Author: jainp1979

cfgmgr/vlanmgr.cpp

@@ -13,10 +13,12 @@ using namespace swss;
 
 #define DOT1Q_BRIDGE_NAME   "Bridge"
 #define VLAN_PREFIX         "Vlan"
+#define SWITCH_STR          "switch"
 #define LAG_PREFIX          "PortChannel"
 #define DEFAULT_VLAN_ID     "1"
 #define DEFAULT_MTU_STR     "9100"
 #define VLAN_HLEN            4
+#define MAX_VLAN_ID          4095
 
 extern MacAddress gMacAddress;
 
@@ -28,6 +30,8 @@ VlanMgr::VlanMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, c
         m_stateLagTable(stateDb, STATE_LAG_TABLE_NAME),
         m_stateVlanTable(stateDb, STATE_VLAN_TABLE_NAME),
         m_stateVlanMemberTable(stateDb, STATE_VLAN_MEMBER_TABLE_NAME),
+        m_appFdbTableProducer(appDb, APP_FDB_TABLE_NAME),
+        m_appSwitchTableProducer(appDb, APP_SWITCH_TABLE_NAME),
         m_appVlanTableProducer(appDb, APP_VLAN_TABLE_NAME),
         m_appVlanMemberTableProducer(appDb, APP_VLAN_MEMBER_TABLE_NAME)
 {
@@ -88,6 +92,10 @@ VlanMgr::VlanMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, c
 
         EXEC_WITH_ERROR_THROW(echo_cmd_backup, res);
     }
+    /* vlan state notification from portsorch */
+    m_VlanStateNotificationConsumer = new swss::NotificationConsumer(appDb, "VLANSTATE");
+    auto vlanStatusNotificatier = new Notifier(m_VlanStateNotificationConsumer, this, "VLANSTATE");
+    Orch::addExecutor(vlanStatusNotificatier);
 }
 
 bool VlanMgr::addHostVlan(int vlan_id)
@@ -101,7 +109,7 @@ bool VlanMgr::addHostVlan(int vlan_id)
       + BASH_CMD + " -c \""
       + BRIDGE_CMD + " vlan add vid " + std::to_string(vlan_id) + " dev " + DOT1Q_BRIDGE_NAME + " self && "
       + IP_CMD + " link add link " + DOT1Q_BRIDGE_NAME
-               + " up"
+               + " down"
                + " name " + VLAN_PREFIX + std::to_string(vlan_id)
                + " address " + gMacAddress.to_string()
                + " type vlan id " + std::to_string(vlan_id) + "\"";
@@ -179,14 +187,29 @@ bool VlanMgr::addHostVlanMember(int vlan_id, const string &port_alias, const str
     // /bin/bash -c "/sbin/ip link set {{port_alias}} master Bridge &&
     //               /sbin/bridge vlan del vid 1 dev {{ port_alias }} &&
     //               /sbin/bridge vlan add vid {{vlan_id}} dev {{port_alias}} {{tagging_mode}}"
-    ostringstream cmds, inner;
-    inner << IP_CMD " link set " << shellquote(port_alias) << " master " DOT1Q_BRIDGE_NAME " && "
-      BRIDGE_CMD " vlan del vid " DEFAULT_VLAN_ID " dev " << shellquote(port_alias) << " && "
-      BRIDGE_CMD " vlan add vid " + std::to_string(vlan_id) + " dev " << shellquote(port_alias) << " " + tagging_cmd;
-    cmds << BASH_CMD " -c " << shellquote(inner.str());
+        
+    const std::string key = std::string("") + "Vlan1|" + port_alias;
 
-    std::string res;
-    EXEC_WITH_ERROR_THROW(cmds.str(), res);
+    if (isVlanMemberStateOk(key)) {
+        ostringstream cmds, inner;
+        inner << IP_CMD " link set " << shellquote(port_alias) << " master " DOT1Q_BRIDGE_NAME " && "
+          BRIDGE_CMD " vlan add vid " + std::to_string(vlan_id) + " dev " << shellquote(port_alias) << " " + tagging_cmd;
+        cmds << BASH_CMD " -c " << shellquote(inner.str());
+
+        std::string res;
+        EXEC_WITH_ERROR_THROW(cmds.str(), res);
+    }
+    else
+    {
+        ostringstream cmds, inner;
+        inner << IP_CMD " link set " << shellquote(port_alias) << " master " DOT1Q_BRIDGE_NAME " && "
+          BRIDGE_CMD " vlan del vid " DEFAULT_VLAN_ID " dev " << shellquote(port_alias) << " && "
+          BRIDGE_CMD " vlan add vid " + std::to_string(vlan_id) + " dev " << shellquote(port_alias) << " " + tagging_cmd;
+        cmds << BASH_CMD " -c " << shellquote(inner.str());
+
+        std::string res;
+        EXEC_WITH_ERROR_THROW(cmds.str(), res);
+    }
 
     return true;
 }
@@ -224,6 +247,137 @@ bool VlanMgr::isVlanMacOk()
     return !!gMacAddress;
 }
 
+void VlanMgr::doSwitchTask(Consumer &consumer)
+{
+    SWSS_LOG_ENTER();
+    auto it = consumer.m_toSync.begin();
+
+    while (it != consumer.m_toSync.end())
+    {
+        KeyOpFieldsValuesTuple t = it->second;
+
+        string key = kfvKey(t);
+        string op = kfvOp(t);
+
+        /* Ensure the key starts with "switch" otherwise ignore */
+        if (key != SWITCH_STR)
+        {
+            SWSS_LOG_NOTICE("Ignoring SWITCH key %s", key.c_str());
+            it = consumer.m_toSync.erase(it);
+            continue;
+        }
+
+        SWSS_LOG_DEBUG("key:switch");
+
+        for (auto i : kfvFieldsValues(t))
+        {
+            if (fvField(i) == "fdb_aging_time")
+            {
+                long agingTime = 0;
+                SWSS_LOG_DEBUG("attribute:fdb_aging_time");
+                if (op == SET_COMMAND)
+                {
+                    SWSS_LOG_DEBUG("operation:set");
+                    agingTime = strtol(fvValue(i).c_str(), NULL, 0);
+                    if (agingTime < 0)
+                    {
+                        SWSS_LOG_ERROR("Invalid fdb_aging_time %s", fvValue(i).c_str());
+                        break;
+                    }
+                    SWSS_LOG_DEBUG("value:%s",fvValue(i).c_str());
+                }
+                else if (op == DEL_COMMAND)
+                {
+                    SWSS_LOG_DEBUG("operation:del");
+                    agingTime = 0;
+                }
+                else
+                {
+                    SWSS_LOG_ERROR("Unknown operation type %s", op.c_str());
+                    break;
+                }
+
+                vector<FieldValueTuple> fvVector;
+                FieldValueTuple aging_time("fdb_aging_time", to_string(agingTime));
+                fvVector.push_back(aging_time);
+                m_appSwitchTableProducer.set(key, fvVector);
+                break;
+            }
+        }
+
+        it = consumer.m_toSync.erase(it);
+    }
+}
+
+void VlanMgr::doFdbTask(Consumer &consumer)
+{
+    auto it = consumer.m_toSync.begin();
+
+    while (it != consumer.m_toSync.end())
+    {
+        KeyOpFieldsValuesTuple t = it->second;
+
+        /* format: <VLAN_name>|<MAC_address> */
+        vector<string> keys = tokenize(kfvKey(t), config_db_key_delimiter, 1);
+        /* keys[0] is vlan as (Vlan10) and keys[1] is mac as (00-00-00-00-00-00) */
+        string op = kfvOp(t);
+
+        /* Ensure the key starts with "Vlan" otherwise ignore */
+        if (strncmp(keys[0].c_str(), VLAN_PREFIX, 4))
+        {
+            SWSS_LOG_ERROR("Invalid key format. No 'Vlan' prefix: %s", keys[0].c_str());
+            it = consumer.m_toSync.erase(it);
+            continue;
+        }
+
+        unsigned long vlan_id;
+        vlan_id = strtoul(keys[0].substr(strlen(VLAN_PREFIX)).c_str(), NULL, 0);
+
+        if ((vlan_id <= 0) || (vlan_id > MAX_VLAN_ID))
+        {
+            SWSS_LOG_ERROR("Invalid key format. Vlan is out of range: %s", keys[0].c_str());
+            it = consumer.m_toSync.erase(it);
+            continue;
+        }
+
+        MacAddress mac = MacAddress(keys[1]);
+
+        string key = VLAN_PREFIX + to_string(vlan_id);
+        key += DEFAULT_KEY_SEPARATOR;
+        key += mac.to_string();
+
+        if (op == SET_COMMAND)
+        {
+            string port;
+            for (auto i : kfvFieldsValues(t))
+            {
+                if (fvField(i) == "port")
+                {
+                    port = fvValue(i);
+                    break;
+                }
+            }
+
+            vector<FieldValueTuple> fvVector;
+            FieldValueTuple p("port", port);
+            fvVector.push_back(p);
+            FieldValueTuple t("type", "static");
+            fvVector.push_back(t);
+
+            m_appFdbTableProducer.set(key, fvVector);
+        }
+        else if (op == DEL_COMMAND)
+        {
+            m_appFdbTableProducer.del(key);
+        }
+        else
+        {
+            SWSS_LOG_ERROR("Unknown operation type %s", op.c_str());
+        }
+        it = consumer.m_toSync.erase(it);
+    }
+}
+
 void VlanMgr::doVlanTask(Consumer &consumer)
 {
     if (!isVlanMacOk())
@@ -296,12 +450,12 @@ void VlanMgr::doVlanTask(Consumer &consumer)
             {
                 /* Set vlan admin status */
                 if (fvField(i) == "admin_status")
-                {
-                    admin_status = fvValue(i);
-                    setHostVlanAdminState(vlan_id, admin_status);
-                    fvVector.push_back(i);
-                }
-                /* Set vlan mtu */
+                {	
+                    admin_status = fvValue(i);	
+                    setHostVlanAdminState(vlan_id, admin_status);	
+                    fvVector.push_back(i);	
+                }	
+                /* Set vlan mtu */	
                 else if (fvField(i) == "mtu")
                 {
                     mtu = fvValue(i);
@@ -593,9 +747,47 @@ void VlanMgr::doTask(Consumer &consumer)
     {
         doVlanMemberTask(consumer);
     }
+    else if (table_name == CFG_FDB_TABLE_NAME)
+    {
+        doFdbTask(consumer);
+    }
+    else if (table_name == CFG_SWITCH_TABLE_NAME)
+    {
+        SWSS_LOG_DEBUG("Table:SWITCH");
+        doSwitchTask(consumer);
+    }
     else
     {
         SWSS_LOG_ERROR("Unknown config table %s ", table_name.c_str());
         throw runtime_error("VlanMgr doTask failure.");
     }
 }
+
+void VlanMgr::doTask(NotificationConsumer &consumer)
+{
+    std::string op;
+    std::string data;
+    std::vector<swss::FieldValueTuple> values;
+
+    if (&consumer != m_VlanStateNotificationConsumer)
+    {
+        SWSS_LOG_WARN("received incorrect notification message");
+        return;
+    }
+
+    consumer.pop(op, data, values);
+
+    unsigned long vlan_id = strtoul(data.substr(strlen(VLAN_PREFIX)).c_str(), NULL, 0);
+
+    SWSS_LOG_NOTICE("vlanmgr received port status notification state %s vlan %s",
+        op.c_str(), data.c_str());
+
+    if (isVlanStateOk(data))
+    {
+        setHostVlanAdminState((int)vlan_id, op);
+    }
+    else
+    {
+        SWSS_LOG_ERROR("received state update for vlan %s not existing", data.c_str());
+    }
+}

cfgmgr/vlanmgr.h

@@ -4,6 +4,7 @@
 #include "dbconnector.h"
 #include "producerstatetable.h"
 #include "orch.h"
+#include "notifier.h"
 
 #include <set>
 #include <map>
@@ -19,14 +20,20 @@ class VlanMgr : public Orch
 
 private:
     ProducerStateTable m_appVlanTableProducer, m_appVlanMemberTableProducer;
+    ProducerStateTable m_appFdbTableProducer;
+    ProducerStateTable m_appSwitchTableProducer;
     Table m_cfgVlanTable, m_cfgVlanMemberTable;
     Table m_statePortTable, m_stateLagTable;
     Table m_stateVlanTable, m_stateVlanMemberTable;
     std::set<std::string> m_vlans;
+    NotificationConsumer* m_VlanStateNotificationConsumer;
 
     void doTask(Consumer &consumer);
+    void doTask(NotificationConsumer &consumer);
     void doVlanTask(Consumer &consumer);
     void doVlanMemberTask(Consumer &consumer);
+    void doFdbTask(Consumer &consumer);
+    void doSwitchTask(Consumer &consumer);
     void processUntaggedVlanMembers(std::string vlan, const std::string &members);
 
     bool addHostVlan(int vlan_id);

cfgmgr/vlanmgrd.cpp

@@ -51,6 +51,8 @@ int main(int argc, char **argv)
         vector<string> cfg_vlan_tables = {
             CFG_VLAN_TABLE_NAME,
             CFG_VLAN_MEMBER_TABLE_NAME,
+            CFG_FDB_TABLE_NAME,
+            CFG_SWITCH_TABLE_NAME,
         };
 
         DBConnector cfgDb("CONFIG_DB", 0);

orchagent/fdborch.cpp

@@ -469,7 +469,7 @@ bool FdbOrch::getPort(const MacAddress& mac, uint16_t vlan, Port& port)
 
     if (!m_portsOrch->getPortByBridgePortId(bridge_port_id, port))
     {
-        SWSS_LOG_ERROR("Failed to get port by bridge port ID 0x%" PRIx64, attr.value.oid);
+        SWSS_LOG_ERROR("Failed to get port by bridge port ID 0x%" PRIx64, bridge_port_id);
         return false;
     }
 
@@ -866,14 +866,21 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, const
     attr.id = SAI_FDB_ENTRY_ATTR_TYPE;
     if (origin == FDB_ORIGIN_VXLAN_ADVERTIZED)
     {
-        attr.value.s32 = (type == "dynamic") ? SAI_FDB_ENTRY_TYPE_STATIC_MACMOVE : SAI_FDB_ENTRY_TYPE_STATIC;
+        attr.value.s32 = SAI_FDB_ENTRY_TYPE_STATIC;
     }
     else
     {
         attr.value.s32 = (type == "dynamic") ? SAI_FDB_ENTRY_TYPE_DYNAMIC : SAI_FDB_ENTRY_TYPE_STATIC;
     }
     attrs.push_back(attr);
 
+    if ((origin == FDB_ORIGIN_VXLAN_ADVERTIZED) && (type == "dynamic"))
+    {
+        attr.id = SAI_FDB_ENTRY_ATTR_ALLOW_MAC_MOVE;
+        attr.value.booldata = true;
+        attrs.push_back(attr);
+    }
+
     attr.id = SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID;
     attr.value.oid = port.m_bridge_port_id;
     attrs.push_back(attr);
@@ -896,7 +903,9 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, const
         attr.value.ipaddr = ipaddr;
         attrs.push_back(attr);
     }
-    else if(macUpdate && (oldOrigin == FDB_ORIGIN_VXLAN_ADVERTIZED) && (origin != oldOrigin))
+    else if(macUpdate 
+            && (oldOrigin == FDB_ORIGIN_VXLAN_ADVERTIZED) 
+            && (origin != oldOrigin))
     {
         /* origin is changed from Remote-advertized to Local-provisioned
          * Remove the end-point ip attribute from fdb entry
@@ -909,7 +918,16 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name, const
         attrs.push_back(attr);
     }
 
-    string key = "Vlan" + to_string(vlan.m_vlan_info.vlan_id) + ":" + entry.mac.to_string();
+    if(macUpdate && (oldOrigin == FDB_ORIGIN_VXLAN_ADVERTIZED)) 
+    {
+        if((origin != oldOrigin)
+           || ((oldType == "dynamic") && (oldType != type)))
+        {
+            attr.id = SAI_FDB_ENTRY_ATTR_ALLOW_MAC_MOVE;
+            attr.value.booldata = false;
+            attrs.push_back(attr);
+        }
+    }
 
 
     if(macUpdate)