[swss] L2 Forwarding Enhancements (sonic-net#1716)

* L2 Forwarding Enhancements

Author: dr412113

orchagent/fdborch.cpp

@@ -874,10 +874,6 @@ void FdbOrch::doTask(NotificationConsumer& consumer)
     {
         if (op == "ALL")
         {
-            /*
-             * so far only support flush all the FDB entries
-             * flush per port and flush per vlan will be added later.
-             */
             status = sai_fdb_api->flush_fdb_entries(gSwitchId, 0, NULL);
             if (status != SAI_STATUS_SUCCESS)
             {
@@ -1080,7 +1076,9 @@ void FdbOrch::updatePortOperState(const PortOperStateUpdate& update)
 
         // Get BVID of each VLAN that this port is a member of
         // and call notifyObserversFDBFlush
-        for (const auto& vlan_member: p.m_vlan_members)
+        vlan_members_t vlan_members;
+        m_portsOrch->getPortVlanMembers(p, vlan_members);
+        for (const auto& vlan_member: vlan_members)
         {
             swss::Port vlan;
             string vlan_alias = VLAN_PREFIX + to_string(vlan_member.first);

orchagent/port.h

@@ -121,6 +121,7 @@ class Port
     VlanInfo            m_vlan_info;
     MacAddress          m_mac;
     sai_object_id_t     m_bridge_port_id = 0;   // TODO: port could have multiple bridge port IDs
+    sai_object_id_t     m_bridge_port_admin_state = 0;   // TODO: port could have multiple bridge port IDs
     sai_vlan_id_t       m_port_vlan_id = DEFAULT_PORT_VLAN_ID;  // Port VLAN ID
     sai_object_id_t     m_rif_id = 0;
     sai_object_id_t     m_vr_id = 0;
@@ -130,7 +131,6 @@ class Port
     sai_object_id_t     m_tunnel_id = 0;
     sai_object_id_t     m_ingress_acl_table_group_id = 0;
     sai_object_id_t     m_egress_acl_table_group_id = 0;
-    vlan_members_t      m_vlan_members;
     sai_object_id_t     m_parent_port_id = 0;
     uint32_t            m_dependency_bitmap = 0;
     sai_port_oper_status_t m_oper_status = SAI_PORT_OPER_STATUS_UNKNOWN;

orchagent/portsorch.cpp

@@ -745,35 +745,15 @@ bool PortsOrch::getPort(sai_object_id_t id, Port &port)
 {
     SWSS_LOG_ENTER();
 
-    for (const auto& portIter: m_portList)
+    auto itr = saiOidToAlias.find(id);
+    if (itr == saiOidToAlias.end())
     {
-        switch (portIter.second.m_type)
-        {
-        case Port::PHY:
-        case Port::SYSTEM:
-            if(portIter.second.m_port_id == id)
-            {
-                port = portIter.second;
-                return true;
-            }
-            break;
-        case Port::LAG:
-            if(portIter.second.m_lag_id == id)
-            {
-                port = portIter.second;
-                return true;
-            }
-            break;
-        case Port::VLAN:
-            if (portIter.second.m_vlan_info.vlan_oid == id)
-            {
-                port = portIter.second;
-                return true;
-            }
-            break;
-        default:
-            continue;
-        }
+        return false;
+    }
+    else
+    {
+        getPort(itr->second, port);
+        return true;
     }
 
     return false;
@@ -813,13 +793,15 @@ bool PortsOrch::getPortByBridgePortId(sai_object_id_t bridge_port_id, Port &port
 {
     SWSS_LOG_ENTER();
 
-    for (auto &it: m_portList)
+    auto itr = saiOidToAlias.find(bridge_port_id);
+    if (itr == saiOidToAlias.end())
     {
-        if (it.second.m_bridge_port_id == bridge_port_id)
-        {
-            port = it.second;
-            return true;
-        }
+        return false;
+    }
+    else
+    {
+        getPort(itr->second, port);
+        return true;
     }
 
     return false;
@@ -2358,6 +2340,7 @@ bool PortsOrch::initPort(const string &alias, const string &role, const int inde
 
                 /* Add port to port list */
                 m_portList[alias] = p;
+                saiOidToAlias[id] = alias;
                 m_port_ref_count[alias] = 0;
                 m_portOidToIndex[id] = index;
 
@@ -3558,7 +3541,7 @@ void PortsOrch::doVlanMemberTask(Consumer &consumer)
             {
                 if (removeVlanMember(vlan, port))
                 {
-                    if (port.m_vlan_members.empty())
+                    if (m_portVlanMember[port.m_alias].empty())
                     {
                         removeBridgePort(port);
                     }
@@ -3661,7 +3644,6 @@ void PortsOrch::doLagTask(Consumer &consumer)
                 switch_id = -1;
             }
 
-            // Create a new LAG when the new alias comes
             if (m_portList.find(alias) == m_portList.end())
             {
                 if (!addLag(alias, lag_id, switch_id))
@@ -4331,6 +4313,7 @@ bool PortsOrch::addBridgePort(Port &port)
         return false;
     }
     m_portList[port.m_alias] = port;
+    saiOidToAlias[port.m_bridge_port_id] = port.m_alias;
     SWSS_LOG_NOTICE("Add bridge port %s to default 1Q bridge", port.m_alias.c_str());
 
     PortUpdate update = { port, true };
@@ -4387,6 +4370,7 @@ bool PortsOrch::removeBridgePort(Port &port)
             return parseHandleSaiStatusFailure(handle_status);
         }
     }
+    saiOidToAlias.erase(port.m_bridge_port_id);
     port.m_bridge_port_id = SAI_NULL_OBJECT_ID;
 
     /* Remove bridge port */
@@ -4460,7 +4444,7 @@ bool PortsOrch::addVlan(string vlan_alias)
         }
     }
 
-    SWSS_LOG_NOTICE("Create an empty VLAN %s vid:%hu", vlan_alias.c_str(), vlan_id);
+    SWSS_LOG_NOTICE("Create an empty VLAN %s vid:%hu vlan_oid:%" PRIx64, vlan_alias.c_str(), vlan_id, vlan_oid);
 
     Port vlan(vlan_alias, Port::VLAN);
     vlan.m_vlan_info.vlan_oid = vlan_oid;
@@ -4470,6 +4454,7 @@ bool PortsOrch::addVlan(string vlan_alias)
     vlan.m_members = set<string>();
     m_portList[vlan_alias] = vlan;
     m_port_ref_count[vlan_alias] = 0;
+    saiOidToAlias[vlan_oid] =  vlan_alias;
 
     return true;
 }
@@ -4478,6 +4463,13 @@ bool PortsOrch::removeVlan(Port vlan)
 {
     SWSS_LOG_ENTER();
 
+    /* If there are still fdb entries associated with the VLAN,
+       return false for retry */
+    if (vlan.m_fdb_count > 0)
+    {
+        SWSS_LOG_NOTICE("VLAN %s still has assiciated FDB entries", vlan.m_alias.c_str());
+        return false;
+    }
     if (m_port_ref_count[vlan.m_alias] > 0)
     {
         SWSS_LOG_ERROR("Failed to remove ref count %d VLAN %s",
@@ -4525,6 +4517,7 @@ bool PortsOrch::removeVlan(Port vlan)
     SWSS_LOG_NOTICE("Remove VLAN %s vid:%hu", vlan.m_alias.c_str(),
             vlan.m_vlan_info.vlan_id);
 
+    saiOidToAlias.erase(vlan.m_vlan_info.vlan_oid);
     m_portList.erase(vlan.m_alias);
     m_port_ref_count.erase(vlan.m_alias);
 
@@ -4614,7 +4607,7 @@ bool PortsOrch::addVlanMember(Port &vlan, Port &port, string &tagging_mode, stri
 
     /* a physical port may join multiple vlans */
     VlanMemberEntry vme = {vlan_member_id, sai_tagging_mode};
-    port.m_vlan_members[vlan.m_vlan_info.vlan_id] = vme;
+    m_portVlanMember[port.m_alias][vlan.m_vlan_info.vlan_id] = vme;
     m_portList[port.m_alias] = port;
     vlan.m_members.insert(port.m_alias);
     m_portList[vlan.m_alias] = vlan;
@@ -4625,6 +4618,12 @@ bool PortsOrch::addVlanMember(Port &vlan, Port &port, string &tagging_mode, stri
     return true;
 }
 
+bool PortsOrch::getPortVlanMembers(Port &port, vlan_members_t &vlan_members)
+{
+    vlan_members = m_portVlanMember[port.m_alias];
+    return true;
+}
+
 bool PortsOrch::addVlanFloodGroups(Port &vlan, Port &port, string end_point_ip)
 {
     SWSS_LOG_ENTER();
@@ -4844,10 +4843,10 @@ bool PortsOrch::removeVlanMember(Port &vlan, Port &port, string end_point_ip)
     }
     sai_object_id_t vlan_member_id;
     sai_vlan_tagging_mode_t sai_tagging_mode;
-    auto vlan_member = port.m_vlan_members.find(vlan.m_vlan_info.vlan_id);
+    auto vlan_member = m_portVlanMember[port.m_alias].find(vlan.m_vlan_info.vlan_id);
 
     /* Assert the port belongs to this VLAN */
-    assert (vlan_member != port.m_vlan_members.end());
+    assert (vlan_member != m_portVlanMember[port.m_alias].end());
     sai_tagging_mode = vlan_member->second.vlan_mode;
     vlan_member_id = vlan_member->second.vlan_member_id;
 
@@ -4862,7 +4861,11 @@ bool PortsOrch::removeVlanMember(Port &vlan, Port &port, string end_point_ip)
             return parseHandleSaiStatusFailure(handle_status);
         }
     }
-    port.m_vlan_members.erase(vlan_member);
+    m_portVlanMember[port.m_alias].erase(vlan_member);
+    if (m_portVlanMember[port.m_alias].empty())
+    {
+        m_portVlanMember.erase(port.m_alias);
+    }
     SWSS_LOG_NOTICE("Remove member %s from VLAN %s lid:%hx vmid:%" PRIx64,
             port.m_alias.c_str(), vlan.m_alias.c_str(), vlan.m_vlan_info.vlan_id, vlan_member_id);
 
@@ -4905,6 +4908,20 @@ bool PortsOrch::addLag(string lag_alias, uint32_t spa_id, int32_t switch_id)
 {
     SWSS_LOG_ENTER();
 
+    auto lagport = m_portList.find(lag_alias);
+    if (lagport != m_portList.end())
+    {
+        /* The deletion of bridgeport attached to the lag may still be 
+         * pending due to fdb entries still present on the lag. Wait
+         * until the cleanup is done.
+         */
+        if (m_portList[lag_alias].m_bridge_port_id != SAI_NULL_OBJECT_ID)
+        {
+            return false;
+        }
+        return true;
+    }
+
     vector<sai_attribute_t> lag_attrs;
     string system_lag_alias = lag_alias;
 
@@ -4956,6 +4973,7 @@ bool PortsOrch::addLag(string lag_alias, uint32_t spa_id, int32_t switch_id)
     lag.m_members = set<string>();
     m_portList[lag_alias] = lag;
     m_port_ref_count[lag_alias] = 0;
+    saiOidToAlias[lag_id] = lag_alias;
 
     PortUpdate update = { lag, true };
     notify(SUBJECT_TYPE_PORT_CHANGE, static_cast<void *>(&update));
@@ -5004,12 +5022,17 @@ bool PortsOrch::removeLag(Port lag)
         SWSS_LOG_ERROR("Failed to remove non-empty LAG %s", lag.m_alias.c_str());
         return false;
     }
-    if (lag.m_vlan_members.size() > 0)
+    if (m_portVlanMember[lag.m_alias].size() > 0)
     {
         SWSS_LOG_ERROR("Failed to remove LAG %s, it is still in VLAN", lag.m_alias.c_str());
         return false;
     }
 
+    if (lag.m_bridge_port_id != SAI_NULL_OBJECT_ID)
+    {
+        return false;
+    }
+
     sai_status_t status = sai_lag_api->remove_lag(lag.m_lag_id);
     if (status != SAI_STATUS_SUCCESS)
     {
@@ -5023,6 +5046,7 @@ bool PortsOrch::removeLag(Port lag)
 
     SWSS_LOG_NOTICE("Remove LAG %s lid:%" PRIx64, lag.m_alias.c_str(), lag.m_lag_id);
 
+    saiOidToAlias.erase(lag.m_lag_id);
     m_portList.erase(lag.m_alias);
     m_port_ref_count.erase(lag.m_alias);
 

orchagent/portsorch.h

@@ -154,6 +154,7 @@ class PortsOrch : public Orch, public Subject
     string m_inbandPortName = "";
     bool isInbandPort(const string &alias);
     bool setVoqInbandIntf(string &alias, string &type);
+    bool getPortVlanMembers(Port &port, vlan_members_t &vlan_members);
 
     bool getRecircPort(Port &p, string role);
 
@@ -226,6 +227,12 @@ class PortsOrch : public Orch, public Subject
     map<set<int>, sai_object_id_t> m_portListLaneMap;
     map<set<int>, tuple<string, uint32_t, int, string, int, string>> m_lanesAliasSpeedMap;
     map<string, Port> m_portList;
+    map<string, vlan_members_t> m_portVlanMember;
+    /* mapping from SAI object ID to Name for faster
+     * retrieval of Port/VLAN from object ID for events
+     * coming from SAI
+     */
+    unordered_map<sai_object_id_t, string> saiOidToAlias;
     unordered_map<sai_object_id_t, int> m_portOidToIndex;
     map<string, uint32_t> m_port_ref_count;
     unordered_set<string> m_pendingPortSet;