Use shared egress ACL table for PFCWD in BRCM DNX platform (sonic-net…

…#3136) * Use shared egress ACL table for PFCWD in BRCM DNX platform. Today PFCWD uses separate egress ACL tables, one table for each traffic class. This may result in ACL resource overflow when other features that also need egress ACL table is enabled at the same time, e.g., MACSEC. ( See sonic-net/sonic-buildimage#17839 for details). The fix is to use shared egress ACL table for PFCWD in BRCM DNX platform.
inspurSDN · Jun 28, 2024 · b02181a · b02181a
1 parent 2137813
commit b02181a
Show file tree

Hide file tree

Showing 8 changed files with 396 additions and 28 deletions.
diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp
@@ -28,6 +28,7 @@ extern sai_switch_api_t* sai_switch_api;
 extern sai_object_id_t   gSwitchId;
 extern PortsOrch*        gPortsOrch;
 extern CrmOrch *gCrmOrch;
+extern SwitchOrch *gSwitchOrch;
 extern string gMySwitchType;
 
 #define MIN_VLAN_ID 1    // 0 is a reserved VLAN ID
@@ -46,6 +47,7 @@ const int TCP_PROTOCOL_NUM = 6; // TCP protocol number
 acl_rule_attr_lookup_t aclMatchLookup =
 {
     { MATCH_IN_PORTS,          SAI_ACL_ENTRY_ATTR_FIELD_IN_PORTS },
+    { MATCH_OUT_PORT,          SAI_ACL_ENTRY_ATTR_FIELD_OUT_PORT },
     { MATCH_OUT_PORTS,         SAI_ACL_ENTRY_ATTR_FIELD_OUT_PORTS },
     { MATCH_SRC_IP,            SAI_ACL_ENTRY_ATTR_FIELD_SRC_IP },
     { MATCH_DST_IP,            SAI_ACL_ENTRY_ATTR_FIELD_DST_IP },
@@ -865,6 +867,23 @@ bool AclRule::validateAddMatch(string attr_name, string attr_value)
             matchData.data.objlist.count = static_cast<uint32_t>(outPorts.size());
             matchData.data.objlist.list = outPorts.data();
         }
+        else if (attr_name == MATCH_OUT_PORT)
+        {
+            auto alias = attr_value;
+            Port port;
+            if (!gPortsOrch->getPort(alias, port))
+            {
+                SWSS_LOG_ERROR("Failed to locate port %s", alias.c_str());
+                return false;
+            }
+            if (port.m_type != Port::PHY)
+            {
+                SWSS_LOG_ERROR("Cannot bind rule to %s: OUT_PORT can only match physical interfaces", alias.c_str());
+                return false;
+            }
+
+            matchData.data.oid = port.m_port_id;
+        }
         else if (attr_name == MATCH_IP_TYPE)
         {
             if (!processIpType(attr_value, matchData.data.u32))
@@ -3201,14 +3220,14 @@ void AclOrch::init(vector<TableConnector>& connectors, PortsOrch *portOrch, Mirr
         m_mirrorV6TableId[stage] = "";
     }
 
-    initDefaultTableTypes();
+    initDefaultTableTypes(platform, sub_platform);
 
     // Attach observers
     m_mirrorOrch->attach(this);
     gPortsOrch->attach(this);
 }
 
-void AclOrch::initDefaultTableTypes()
+void AclOrch::initDefaultTableTypes(const string& platform, const string& sub_platform)
 {
     SWSS_LOG_ENTER();
 
@@ -3295,12 +3314,26 @@ void AclOrch::initDefaultTableTypes()
             .build()
     );
 
-    addAclTableType(
-        builder.withName(TABLE_TYPE_PFCWD)
-            .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT)
-            .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_TC))
-            .build()
-    );
+    // Use SAI_ACL_BIND_POINT_TYPE_SWITCH in BRCM DNX platforms to use shared egress ACL table for PFCWD.
+    if (platform == BRCM_PLATFORM_SUBSTRING && sub_platform == BRCM_DNX_PLATFORM_SUBSTRING)
+    {
+        addAclTableType(
+            builder.withName(TABLE_TYPE_PFCWD)
+                .withBindPointType(SAI_ACL_BIND_POINT_TYPE_SWITCH)
+                .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_TC))
+                .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_OUT_PORT))
+                .build()
+        );
+    }
+    else
+    {
+        addAclTableType(
+            builder.withName(TABLE_TYPE_PFCWD)
+                .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT)
+                .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_TC))
+                .build()
+        );
+    }
 
     addAclTableType(
         builder.withName(TABLE_TYPE_DROP)
@@ -3974,6 +4007,20 @@ bool AclOrch::addAclTable(AclTable &newTable)
             m_mirrorV6TableId[table_stage] = table_id;
         }
 
+        // We use SAI_ACL_BIND_POINT_TYPE_SWITCH for PFCWD table in DNX platform.
+        // This bind type requires to bind the table to switch.
+        string platform = getenv("platform") ? getenv("platform") : "";
+        string sub_platform = getenv("sub_platform") ? getenv("sub_platform") : "";
+        if (platform == BRCM_PLATFORM_SUBSTRING && sub_platform == BRCM_DNX_PLATFORM_SUBSTRING &&
+            newTable.type.getName() == TABLE_TYPE_PFCWD)
+        {
+            if(!gSwitchOrch->bindAclTableToSwitch(ACL_STAGE_EGRESS, newTable.getOid()))
+            {
+                return false;
+            }
+            newTable.bindToSwitch = true;
+        }
+
         return true;
     }
     else
@@ -3998,6 +4045,18 @@ bool AclOrch::removeAclTable(string table_id)
     bool suc = m_AclTables[table_oid].clear();
     if (!suc) return false;
 
+    // Unbind table from switch if needed.
+    AclTable &table = m_AclTables.at(table_oid);
+    if (table.bindToSwitch)
+    {
+        // Only bind egress table to switch for now.
+        assert(table->stage == ACL_STAGE_EGRESS);
+        if(!gSwitchOrch->unbindAclTableFromSwitch(ACL_STAGE_EGRESS, table.getOid()))
+        {
+            return false;
+        }
+    }
+
     if (deleteUnbindAclTable(table_oid) == SAI_STATUS_SUCCESS)
     {
         auto stage = m_AclTables[table_oid].stage;
@@ -5111,4 +5170,3 @@ void AclOrch::removeAllAclRuleStatus()
         m_aclRuleStateTable.del(key);
     }
 }
-
diff --git a/orchagent/aclorch.h b/orchagent/aclorch.h
@@ -23,6 +23,7 @@
 
 #define RULE_PRIORITY           "PRIORITY"
 #define MATCH_IN_PORTS          "IN_PORTS"
+#define MATCH_OUT_PORT          "OUT_PORT"
 #define MATCH_OUT_PORTS         "OUT_PORTS"
 #define MATCH_SRC_IP            "SRC_IP"
 #define MATCH_DST_IP            "DST_IP"
@@ -451,6 +452,9 @@ class AclTable
     // Set to store the not configured ACL table port alias
     set<string> pendingPortSet;
 
+    // Is the ACL table bound to switch?
+    bool bindToSwitch = false;
+
 private:
     sai_object_id_t m_oid = SAI_NULL_OBJECT_ID;
     AclOrch *m_pAclOrch = nullptr;
@@ -530,7 +534,7 @@ class AclOrch : public Orch, public Observer
     void doAclRuleTask(Consumer &consumer);
     void doAclTableTypeTask(Consumer &consumer);
     void init(vector<TableConnector>& connectors, PortsOrch *portOrch, MirrorOrch *mirrorOrch, NeighOrch *neighOrch, RouteOrch *routeOrch);
-    void initDefaultTableTypes();
+    void initDefaultTableTypes(const string& platform, const string& sub_platform);
 
     void queryMirrorTableCapability();
     void queryAclActionCapability();

diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp
@@ -676,7 +676,25 @@ bool OrchDaemon::init()
             SAI_QUEUE_ATTR_PAUSE_STATUS,
         };
 
-        if(gSwitchOrch->checkPfcDlrInitEnable())
+        bool pfcDlrInit = gSwitchOrch->checkPfcDlrInitEnable();
+
+        // Override pfcDlrInit if needed, and this change is only for PFC tests.
+        if(getenv("PFC_DLR_INIT_ENABLE"))
+        {
+            string envPfcDlrInit = getenv("PFC_DLR_INIT_ENABLE");
+            if(envPfcDlrInit == "1")
+            {
+                pfcDlrInit = true;
+                SWSS_LOG_NOTICE("Override PfcDlrInitEnable to true");
+            }
+            else if(envPfcDlrInit == "0")
+            {
+                pfcDlrInit = false;
+                SWSS_LOG_NOTICE("Override PfcDlrInitEnable to false");
+            }
+        }
+
+        if(pfcDlrInit)
         {
             m_orchList.push_back(new PfcWdSwOrch<PfcWdDlrHandler, PfcWdDlrHandler>(
                         m_configDb,

diff --git a/orchagent/pfcactionhandler.cpp b/orchagent/pfcactionhandler.cpp
@@ -344,19 +344,58 @@ PfcWdAclHandler::PfcWdAclHandler(sai_object_id_t port, sai_object_id_t queue,
 
     // Egress table/rule creation
     table_type = TABLE_TYPE_PFCWD;
-    m_strEgressTable = "EgressTable_PfcWdAclHandler_" + queuestr;
-    found = m_aclTables.find(m_strEgressTable);
-    if (found == m_aclTables.end())
+
+    // Use shared egress acl table for BRCM DNX platform.
+    string platform = getenv("platform") ? getenv("platform") : "";
+    string sub_platform = getenv("sub_platform") ? getenv("sub_platform") : "";
+    shared_egress_acl_table = (platform == BRCM_PLATFORM_SUBSTRING &&
+                               sub_platform == BRCM_DNX_PLATFORM_SUBSTRING);
+
+    if (shared_egress_acl_table)
     {
-        // First time of handling PFC for this queue, create ACL table, and bind
-        createPfcAclTable(port, m_strEgressTable, false);
-        shared_ptr<AclRulePacket> newRule = make_shared<AclRulePacket>(gAclOrch, m_strRule, m_strEgressTable);
-        createPfcAclRule(newRule, queueId, m_strEgressTable, port);
+        Port p;
+        if (!gPortsOrch->getPort(port, p))
+        {
+            SWSS_LOG_ERROR("Failed to get port structure from port oid 0x%" PRIx64, port);
+            return;
+        }
+        m_strEgressRule = "Egress_Rule_PfcWdAclHandler_" + p.m_alias + "_" + queuestr;
+        m_strEgressTable = "EgressTable_PfcWdAclHandler";
+        found = m_aclTables.find(m_strEgressTable);
+        if (found == m_aclTables.end())
+        {
+            // First time of handling PFC, create ACL table and also ACL rule.
+            createPfcAclTable(port, m_strEgressTable, false);
+            shared_ptr<AclRulePacket> newRule = make_shared<AclRulePacket>(gAclOrch, m_strEgressRule, m_strEgressTable);
+            createPfcAclRule(newRule, queueId, m_strEgressTable, port);
+        }
+        else
+        {
+            // ACL table already exists. Add ACL rule if needed.
+            AclRule* rule = gAclOrch->getAclRule(m_strEgressTable, m_strEgressRule);
+            if (rule == nullptr)
+            {
+                shared_ptr<AclRulePacket> newRule = make_shared<AclRulePacket>(gAclOrch, m_strEgressRule, m_strEgressTable);
+                createPfcAclRule(newRule, queueId, m_strEgressTable, port);
+            }
+        }
     }
     else
     {
-        // Otherwise just bind ACL table with the port
-        found->second.bind(port);
+        m_strEgressTable = "EgressTable_PfcWdAclHandler_" + queuestr;
+        found = m_aclTables.find(m_strEgressTable);
+        if (found == m_aclTables.end())
+        {
+            // First time of handling PFC for this queue, create ACL table, and bind
+            createPfcAclTable(port, m_strEgressTable, false);
+            shared_ptr<AclRulePacket> newRule = make_shared<AclRulePacket>(gAclOrch, m_strRule, m_strEgressTable);
+            createPfcAclRule(newRule, queueId, m_strEgressTable, port);
+        }
+        else
+        {
+            // Otherwise just bind ACL table with the port
+            found->second.bind(port);
+        }
     }
 }
 
@@ -382,8 +421,20 @@ PfcWdAclHandler::~PfcWdAclHandler(void)
         gAclOrch->updateAclRule(m_strIngressTable, m_strRule, MATCH_IN_PORTS, &port, RULE_OPER_DELETE);
     } 
 
-    auto found = m_aclTables.find(m_strEgressTable);
-    found->second.unbind(port);
+    if (shared_egress_acl_table)
+    {
+        rule = gAclOrch->getAclRule(m_strEgressTable, m_strEgressRule);
+        if (rule == nullptr)
+        {
+            SWSS_LOG_THROW("Egress ACL Rule does not exist for rule %s", m_strEgressRule.c_str());
+        }
+        gAclOrch->removeAclRule(m_strEgressTable, m_strEgressRule);
+    }
+    else
+    {
+        auto found = m_aclTables.find(m_strEgressTable);
+        found->second.unbind(port);
+    }
 }
 
 void PfcWdAclHandler::clear()
@@ -418,7 +469,11 @@ void PfcWdAclHandler::createPfcAclTable(sai_object_id_t port, string strTable, b
         return;
     }
 
-    aclTable.link(port);
+    // Link port only for ingress ACL table or unshared egress ACL table.
+    if (ingress || !shared_egress_acl_table)
+    {
+        aclTable.link(port);
+    }
 
     if (ingress) 
     {
@@ -452,18 +507,24 @@ void PfcWdAclHandler::createPfcAclRule(shared_ptr<AclRulePacket> rule, uint8_t q
     attr_value = to_string(queueId);
     rule->validateAddMatch(attr_name, attr_value);
 
-    // Add MATCH_IN_PORTS as match criteria for ingress table
-    if (strTable == INGRESS_TABLE_DROP) 
+    // Add MATCH_IN_PORTS as match criteria for ingress table and MATCH_OUT_PORT as match creiteria for shared egress table.
+    if (strTable == INGRESS_TABLE_DROP || shared_egress_acl_table)
     {
         Port p;
-        attr_name = MATCH_IN_PORTS;
-
+        if (strTable == INGRESS_TABLE_DROP) 
+        {
+            attr_name = MATCH_IN_PORTS;
+        }
+        else if (shared_egress_acl_table) {
+            attr_name = MATCH_OUT_PORT;
+        }
+
         if (!gPortsOrch->getPort(portOid, p))
         {
             SWSS_LOG_ERROR("Failed to get port structure from port oid 0x%" PRIx64, portOid);
             return;
         }
-
+    
         attr_value = p.m_alias;
         rule->validateAddMatch(attr_name, attr_value);
     }

diff --git a/orchagent/pfcactionhandler.h b/orchagent/pfcactionhandler.h
@@ -107,9 +107,12 @@ class PfcWdAclHandler: public PfcWdLossyHandler
         // class shared dict: ACL table name -> ACL table
         static std::map<std::string, AclTable> m_aclTables;
 
+        bool shared_egress_acl_table = false;
+
         string m_strIngressTable;
         string m_strEgressTable;
         string m_strRule;
+        string m_strEgressRule;
         void createPfcAclTable(sai_object_id_t port, string strTable, bool ingress);
         void createPfcAclRule(shared_ptr<AclRulePacket> rule, uint8_t queueId, string strTable, sai_object_id_t port);
         void updatePfcAclRule(shared_ptr<AclRule> rule, uint8_t queueId, string strTable, vector<sai_object_id_t> port);

diff --git a/orchagent/switchorch.cpp b/orchagent/switchorch.cpp
@@ -1515,3 +1515,51 @@ bool SwitchOrch::querySwitchCapability(sai_object_type_t sai_object, sai_attr_id
         }
     }
 }
+
+// Bind ACL table (with bind type switch) to switch
+bool SwitchOrch::bindAclTableToSwitch(acl_stage_type_t stage, sai_object_id_t table_id)
+{
+    sai_attribute_t attr;
+    if ( stage == ACL_STAGE_INGRESS ) {
+        attr.id = SAI_SWITCH_ATTR_INGRESS_ACL;
+    } else {
+        attr.id = SAI_SWITCH_ATTR_EGRESS_ACL;
+    }
+    attr.value.oid = table_id;
+    sai_status_t status = sai_switch_api->set_switch_attribute(gSwitchId, &attr);
+    string stage_str = (stage == ACL_STAGE_INGRESS) ? "ingress" : "egress";
+    if (status == SAI_STATUS_SUCCESS)
+    {
+        SWSS_LOG_NOTICE("Bind %s acl table %" PRIx64" to switch", stage_str.c_str(), table_id);
+        return true;
+    }
+    else
+    {
+        SWSS_LOG_ERROR("Failed to bind %s acl table %" PRIx64" to switch", stage_str.c_str(), table_id);
+        return false;
+    }
+}
+
+// Unbind ACL table from swtich
+bool SwitchOrch::unbindAclTableFromSwitch(acl_stage_type_t stage,sai_object_id_t table_id)
+{
+    sai_attribute_t attr;
+    if ( stage == ACL_STAGE_INGRESS ) {
+        attr.id = SAI_SWITCH_ATTR_INGRESS_ACL;
+    } else {
+        attr.id = SAI_SWITCH_ATTR_EGRESS_ACL;
+    }
+    attr.value.oid = SAI_NULL_OBJECT_ID;
+    sai_status_t status = sai_switch_api->set_switch_attribute(gSwitchId, &attr);
+    string stage_str = (stage == ACL_STAGE_INGRESS) ? "ingress" : "egress";
+    if (status == SAI_STATUS_SUCCESS)
+    {
+        SWSS_LOG_NOTICE("Unbind %s acl table %" PRIx64" to switch", stage_str.c_str(), table_id);
+        return true;
+    }
+    else
+    {
+        SWSS_LOG_ERROR("Failed to unbind %s acl table %" PRIx64" to switch", stage_str.c_str(), table_id);
+        return false;
+    }
+}
diff --git a/orchagent/switchorch.h b/orchagent/switchorch.h
@@ -65,6 +65,9 @@ class SwitchOrch : public Orch
             return (m_fatalEventCount != 0);
         }
 
+    bool bindAclTableToSwitch(acl_stage_type_t stage, sai_object_id_t table_id);
+    bool unbindAclTableFromSwitch(acl_stage_type_t stage, sai_object_id_t table_id);
+
 private:
     void doTask(Consumer &consumer);
     void doTask(swss::SelectableTimer &timer);